On Fri, Nov 25, 2011 at 07:33:18PM -0500, Nir Krakauer wrote:
> I've made a number of changes to line_min for compatibility with the
> current Octave command set as well as for more efficient operation.
> Please see the attached.
> 
> Best,
> 
> Nir

Nir,

a few issues. First of all:

- Why is 'h' randomized? Even if there should be a good reason, I think this
cannot be done inside 'line_min'. Rather, make 'h' configurable so that users
can randomize it 'from outside'.

- Are you sure you don't break someones code by limiting number of evaluations
to 30? Shouldn't this rather be made configurable, too?

And after the above is clear:

- You seem to have started your changes from an old version and to have never
commited your changes. Meanwhile, others have done some of the changes in a
different way. 'splice' is currently not used. You should look at the current
code (in SVN) and re-make your changes there (a.o., there should be no need
to change handling of 'args' now). What remains of your changes is limiting
the number of function evaluations, handling of 'h' (arguable!), and checking
for improvement of function result.

- I think you should only list those changes in the comments which actually
are made to the code in SVN now.

- Minor issue: The preferred style of Octave is leaving a space between
fuction name and parantheses (e.g. sum ()) (you have deleted the space at
some point(s)).

Olaf


------------------------------------------------------------------------------
All the data continuously generated in your IT infrastructure 
contains a definitive record of customers, application performance, 
security threats, fraudulent activity, and more. Splunk takes this 
data and makes sense of it. IT sense. And common sense.
http://p.sf.net/sfu/splunk-novd2d
_______________________________________________
Octave-dev mailing list
Octave-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/octave-dev

Reply via email to