tir, 25 05 2010 kl. 00:54 +0200, skrev Alois Schlögl:
> > A few minor questions:
> >
> >> -while niter++ <= maxout && nev(1) < maxev
> >> +while niter <= maxout
> >> + niter += 1;
> >> + if nev(1) < maxev, break; end;
> >
> > I don't understand this change. I guess you moved the ++ operation to
> > help 'oct2mat' (which is fine by me),
>
> Good, thanks.
>
>
> but why didn't you just write
> >
> > while (niter <= maxout && nev (1) < maxev)
> > niter ++;
>
>
> When the condition is false, niter is not incremented anymore. So, niter
> might be off by 1 when the while loop terminates.
>
> niter is checked after the loop has terminated, and the loop can also
> terminate by some break statements. So, there might be a slight chance
> that this difference is relevant.
Ohhh, tricky. I guess you thought more about this than I did :-)
> >> - c = 1 + sum ((x-y)(:)'*z*((x-y)(:)));
> >> + c = 1 + sum (vec(x-y)'*z*(vec(x-y)));
> >
> > What is the purpose of this (and similar) change?
>
>
> Its again an issue with oct2mat, which can not deal well with (x-y)(:),
> especially if these are used within more complicated constructs. Using
> vec() avoids this problem.
Can't 'oct2mat' just call 'vec' for these types of expressions? It just
seems to me that changing code to fit 'oct2mat' is not the right
approach (it should be the other way around; that way 'oct2mat' would be
more useful). In this case it might even have an impact on performance,
though I doubt it matters.
> >> Index: main/optim/inst/minimize.m
> >> ===================================================================
> >> --- main/optim/inst/minimize.m (revision 7325)
> >> +++ main/optim/inst/minimize.m (revision 7326)
> >> @@ -134,23 +134,7 @@
> >> "isz", nan);
> >>
> >> if nargin == 3 # Accomodation to struct and list
> >> optional
> >> - va_arg_cnt = 1; # args
> >> - tmp = nth (varargin, va_arg_cnt++);
> >> -
> >> - if isstruct (tmp)
> >> - opls = list ();
> >> - for [v,k] = tmp # Treat separately unary and binary opts
> >> - if findstr ([" ",k," "],op0)
> >> - opls = append (opls, k);
> >> - else
> >> - opls = append (opls, k, v);
> >> - end
> >> - end
> >> - elseif is_list (tmp)
> >> - opls = tmp;
> >> - else
> >> - opls = list (tmp);
> >> - end
> >> + opls = varargin{1};
> >
> > I am most certainly missing the something here, but this change just
> > looks odd to me. Can all of the above really be reduced to one line of
> > code?
>
> The line of thought was that append, is_list and list are all
> list-specific commands. Since lists are not supported anymore in v3.4,
> this would be obsolete. Now, I see that this was a short-sighted view.
> This part should convert various types of input arguments into cell
> arrays. Thanks for pointing this out.
>
> I found some other issues, that were not properly addressed in the
> list->cell conversion. The changes are summarized in the patch below, I
> committed them already.
Ok. I don't really know the 'optim' code so it is hard for me to comment
(I just commented on what I stumbled upon). It would be great if others
(more knowledgeable 'optim' people) could comment on the changes. It
would really be great if 'optim' could work with Octave 3.4 (these
changes are needed for this).
Søren
------------------------------------------------------------------------------
_______________________________________________
Octave-dev mailing list
Octave-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/octave-dev
