Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enh: made l-bfgs-b parameter for the maximum number of line search steps accessible in python #5105

Merged
merged 1 commit into from Aug 16, 2015

Conversation

Projects
None yet
4 participants
@fabian-paul
Copy link
Contributor

fabian-paul commented Aug 1, 2015

I sometimes found it useful for my minimization problems to increase the maximum number of line searches in the l-bfgs-b algorithm. The parameter was hard-coded in the Fortran source. I made it accessible as an optional argument.
I found it a bit difficult to come up with a good unit test. It's hard to find a function that is at the same time simple and where the minimization requires more than 20 steps (the default). I would appreciate, if you could help me here.

@josef-pkt

This comment has been minimized.

Copy link
Member

josef-pkt commented Aug 3, 2015

@fabian-paul You could try the example in #3581. I don't remember if I checked with l-bfgs-b

@ev-br

This comment has been minimized.

Copy link
Member

ev-br commented Aug 6, 2015

One options for a smoke test would be to limit the number of steps:

In [27]: fmin_l_bfgs_b(rosen, np.array([2.]*10), approx_grad=True, maxls=1)
Out[27]: 
(array([ 0.99463602,  1.00035832,  1.00115208,  1.00884793,  1.01657323,
         1.03195997,  1.06011627,  1.11885508,  1.24804547,  1.54354017]),
 181100.37793466315,
 {'funcalls': 231,
  'grad': array([-4.41000256,  2.03803379, -2.53061761,  1.81080393,  0.38712604,
          1.76341249,  1.27181887,  0.93616264,  6.76550753, -2.81546318]),
  'nit': 18,
  'task': 'ABNORMAL_TERMINATION_IN_LNSRCH',
  'warnflag': 2})

It seems to me that this new parameters needs to be mentioned in the optimize docs: http://docs.scipy.org/doc/scipy-dev/reference/optimize.minimize-lbfgsb.html

EDIT: I've no idea if these docs are autogenerated from the fmin_ docstrings or not.

@@ -20,6 +20,7 @@ python module _lbfgsb ! in
logical dimension(4),intent(inout) :: lsave
integer dimension(44),intent(inout) :: isave
double precision dimension(29),intent(inout) :: dsave
integer intent(in) :: maxls

This comment has been minimized.

Copy link
@ev-br

ev-br Aug 6, 2015

Member

I'd add check(maxls > 0) here. Or even do basic sanity checking at the python level.

@fabian-paul

This comment has been minimized.

Copy link
Contributor Author

fabian-paul commented Aug 9, 2015

Thanks, Evgeni! I've added your suggestions.
About the documentation, I don't know either how this is updated...

@fabian-paul

This comment has been minimized.

Copy link
Contributor Author

fabian-paul commented Aug 9, 2015

@josef-pkt : issue #3581 doesn't really help. I can't reproduce the problem that is described there with the current scipy version (0.17.0.dev0+0af8320).

@@ -108,6 +108,8 @@ def fmin_l_bfgs_b(func, x0, fprime=None, args=(),
callback : callable, optional
Called after each iteration, as ``callback(xk)``, where ``xk`` is the
current parameter vector.
maxls : int, optional
Maximum number of line search steps (per iteration).

This comment has been minimized.

Copy link
@ev-br

ev-br Aug 9, 2015

Member

Nitpick: better add "default is 20" or equivalent.

@ev-br

This comment has been minimized.

Copy link
Member

ev-br commented Aug 9, 2015

Looks good to me modulo a nitpick.
@fabian-paul could you squash it to one commit and prepend ENH to the commit message, if you are at ease with git.

@fabian-paul

This comment has been minimized.

Copy link
Contributor Author

fabian-paul commented Aug 9, 2015

Ok so far?

@dlax dlax added this to the 0.17.0 milestone Aug 16, 2015

dlax added a commit that referenced this pull request Aug 16, 2015

Merge pull request #5105 from fabian-paul/lbfgsb_maximum_linesearch_p…
…arameter

enh: made l-bfgs-b parameter for the maximum number of line search steps accessible in python

@dlax dlax merged commit 2b3fc57 into scipy:master Aug 16, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@fabian-paul

This comment has been minimized.

Copy link
Contributor Author

fabian-paul commented Aug 17, 2015

Thank you all for reviewing and merging!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.