[CHANGESET] gmres (Re: Last review of PROJECTS file before release)

[John W. Eaton]

On 10-Feb-2011, c. wrote:

| 
| On 5 Feb 2011, at 15:08, address@hidden wrote:
| 
| > 2/4/11
| > 
| > The PROJECTS file was put up on the Octave Wiki by Jordi so that anyone
| > could cross off or add new items to the list.  I will take it down tomorrow
| > and commit the changes back to Mercurial.
| > 
| > However, it would be useful if developers could actually take a look at the
| > list.  I'm pretty sure some of the items are no longer necessary.  The very
| > first item, for example, is to improve logm.  But, I happen to know that
| > Jaroslav, M. Caliari, R.T. Guy, and myself already did quite a bit of work
| > to convert it from using eig to a Schur decomposition and Pade Approximants.
| > 
| > The link to the wiki page is http://wiki.octave.org/wiki.pl?Projects
| > 
| > --Rik
| 
| Hi,
| 
| Looking at the PROJECTS I see that gmres is listed there as a missing 
function to be implemented.
| As I had in Octave-Forge a version of the algorithm I had implemented some 
time ago with help from Jaroslav 
| I changed it a bit to make its interface more matlab compatible and prepared 
a changeset against tip.
| 
| The changeset also adds the mgorth function that implements modified 
Gram-Schmidt orthogonalization, 
| it was initially a private function but Jaroslav pointed out that it may be 
useful in its own right.
| 
| Could someone pleas check if this looks OK and maybe push it?

Here are some very minor comments.  I'd say it is OK to push these
changes now, but would prefer if you addressed the comments before
pushing rather than after.

| @@ -188,3 +188,5 @@
|  @DOCSTRING(bicgstab)
|  
|  @DOCSTRING(cgs)
| +
| address@hidden(gmres)

Is there some reason to not document mgorth in the manual?

| +## Copyright (C) 2009,2010,2011 Carlo de Falco

To be consistent with other files in Octave now, please write
2009-2011.  That will make it easier for us to update this
automatically in the future.

| +##  This program is free software; you can redistribute it and/or modify
| +##  [...]
| +##  along with this program; if not, write to the Free Software
| +##  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
USA

Please update this to use the statement for Octave. It says "This file
is part of Octave" and ends with a URL instead of a physical address.

| +## @deftypefn {Function File} @
| +##   address@hidden, @var{flag}, @var{relres}, @var{iter}, @var{resvec}] =} 
pgmres (@var{A}, @var{b}, @var{m}, @var{rtol}, @var{maxit}, @var{M1}, @var{M2}, 
@var{x0})
| +##
| +##   [x, resids] = pgmres (A, b, x0, rtol, maxit, m, P)

Shouldn't this line use deftypefnx and have some markup for the
variable names?

| +%!shared A, b, dim
| +%!test
| +%! dim = 100;
| +%! A = spdiags ([-ones(dim,1) 2*ones(dim,1) ones(dim,1)], [-1:1], dim, dim);
| +%! b =  ones(dim, 1); 
| +%! x = gmres (A, b, 10, 1e-10, dim, @(x) x./diag(A), [],  b);
| +%! assert(x, A\b, 1e-9*norm(x,inf))

Thanks for including tests!

| +              retval(0) = x;
| +              retval(1) = h;

Please write

  retval(1) = h;
  retval(0) = x;

Writing in the reverse order means that the array that is used to
implement the octave_value_list object is only resized once rather
than twice.

jwe