[Octave-bug-tracker] [bug #31742] sqp: incorrect order of removal of Inf bounds

[Rik]

Follow-up Comment #5, bug #31742 (project octave):

I finally had time this weekend to review your patch.  I think your second
approach is the the correct one.  I have a few comments that I hope you can
incorporate into a final patch.

First, using scalars for upper and lower bounds no longer works.  That
functionality could be restored by always extending any scalar bound to a
vector the size of x.  But perhaps you want to take a look at it and see if
there is a better approach?

Second, the original author seems to have used global variables to pass
information to subfunctions.  Presumably this was done for efficiency since
each variable in an ordinary function call would have to be pushed onto the
stack and popped back off.  I would recommend adopting this strategy for your
function declarations:

     ci_fun = @ (x) cf_ub_lb (x, lower_bounds_idx, upper_bounds_idx);
     ci_grd = @ (x) cigrad_ub_lb (x, bounds_grad);
                to
     ci_fun = @cf_ub_lb;
     ci_grd = @cigrad_ub_lb;

The original author used __sqp_lb__ for the lower bounds so something like
__sqp_lbidx__ would make sense.

Third, I know the original author used isinf() but it is less efficient and
less clear than using a direct test for Inf.  I would change the following
code.

(isinf (lb) & lb < 0)
        to
(lb == -Inf)

The original construction requires iterating over the length of lb 3 times
(isinf, lb < 0, '&') whereas the second construction makes only one pass and
clearly shows what you are testing for.



    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?31742>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/