ENH minimize, minimize_scalar: Add support for user-provided methods #3369

Merged
merged 1 commit into from Feb 27, 2014

Conversation

Projects
None yet
6 participants
Contributor

pasky commented Feb 22, 2014

I'm using the excellent scipy.optimize infrastructure for research, experiments and benchmarking of some custom local minimization strategies. However, without this simple change, I have no easy way to plug my minimizer to basinhopping to overload it for global minimization as well; first I added a direct override possibility in PR #3321, but it has been pointed out that a more generic mechanism that could cover other cases would be nicer. A global registry of custom method names has been another idea, implemented in PR #3333 and it has its merits, but it has been deemed that overally a simpler and nicer way is simply passing a callable as the method parameter, instead of a string.

Therefore, this change does just that, checking if method is a callable instead of a string and in that case just going ahead and calling it - both in minimize and minimize_scalar of scipy.optimize. It now also includes examples in the documentation and test suite checks.

Coverage Status

Coverage remained the same when pulling d17d264 on pasky:minimize-custom into 1b34692 on scipy:master.

@dlax dlax commented on the diff Feb 23, 2014

scipy/optimize/_minimize.py
@@ -328,11 +394,11 @@ def minimize(fun, x0, args=(), method=None, jac=None, hess=None,
warn('Method %s does not use gradient information (jac).' % method,
RuntimeWarning)
# - hess
- if meth not in ('newton-cg', 'dogleg', 'trust-ncg') and hess is not None:
+ if meth not in ('newton-cg', 'dogleg', 'trust-ncg', '_custom') and hess is not None:
@dlax

dlax Feb 23, 2014

Member

What's the point of this addition? A custom method could use hess or hessp. If something needs to be checked I'd say it should be in the method function itself.

@pasky

pasky Feb 23, 2014

Contributor

On Sun, Feb 23, 2014 at 12:00:38PM -0800, Denis Laxalde wrote:

@@ -328,11 +394,11 @@ def minimize(fun, x0, args=(), method=None, jac=None, hess=None,
# - hess

  • if meth not in ('newton-cg', 'dogleg', 'trust-ncg') and hess is not None:
  • if meth not in ('newton-cg', 'dogleg', 'trust-ncg', '_custom') and hess is not None:

What's the point of this addition? A custom method could use hess or hessp. If something needs to be checked I'd say it should be in the method function itself.

Exactly - a custom method could use hess, therefore it is
inappropriate to print a warning when hess is passed that asserts that
the method does not use Hessian. I agree with your second sentence,
that is why I'm exempting _custom from these checks.

@dlax

dlax Feb 23, 2014

Member

Ah yes. I missed the not in.

@dlax dlax commented on the diff Feb 23, 2014

scipy/optimize/_minimize.py
@@ -376,10 +442,14 @@ def minimize(fun, x0, args=(), method=None, jac=None, hess=None,
options.setdefault('ftol', tol)
if meth in ['bfgs', 'cg', 'l-bfgs-b', 'tnc', 'dogleg', 'trust-ncg']:
options.setdefault('gtol', tol)
- if meth in ['cobyla']:
+ if meth in ['cobyla', '_custom']:
@dlax

dlax Feb 23, 2014

Member

Same remark here. Just leave options as it is passed to minimize.

@pasky

pasky Feb 23, 2014

Contributor

On Sun, Feb 23, 2014 at 12:03:16PM -0800, Denis Laxalde wrote:

@@ -376,10 +442,14 @@ def minimize(fun, x0, args=(), method=None, jac=None, hess=None,

  •    if meth in ['cobyla']:
    
  •    if meth in ['cobyla', '_custom']:
         options.setdefault('tol', tol)
    

Same remark here. Just leave options as it is passed to minimize.

In that case, the tol commandline parameter of minimize will be
simply ignored in case of custom functions, even though it is used
for all builtin functions. Isn't that behavior rather surprising to
the user?

@dlax

dlax Feb 23, 2014

Member

In fact it seems to me that tol should just not be passed to the custom method as it is a convenience parameter for "standard" solvers.

@dlax

dlax Feb 23, 2014

Member

2014-02-23 21:07 GMT+01:00 Petr Baudis notifications@github.com:

In that case, the tol commandline parameter of minimize will be simply
ignored in case of custom functions, even though it is used for all builtin
functions. Isn't that behavior rather surprising to the user?

Yes, maybe you're right. Keep it as is.

@dlax dlax commented on an outdated diff Feb 23, 2014

scipy/optimize/_minimize.py
options.setdefault('tol', tol)
- if meth == 'nelder-mead':
+ if meth == '_custom':
+ return method(fun, x0, args=args, jac=jac, hess=hess, hessp=hessp,
+ bounds=bounds,constraints=constraints, callback=callback,
@dlax

dlax Feb 23, 2014

Member
  • align with parentheses
  • missing space after bounds,

@dlax dlax commented on an outdated diff Feb 23, 2014

scipy/optimize/_minimize.py
@@ -486,6 +557,23 @@ def minimize_scalar(fun, bracket=None, bounds=None, args=(),
Method *Bounded* can perform bounded minimization. It uses the Brent
method to find a local minimum in the interval x1 < xopt < x2.
+ **Custom minimizers**
+
+ It may be useful to pass a custom minimization method, for example
+ when using some library frontend to minimize_scalar. You can simply
+ pass a callable as the ``method`` parameter.
+
+ The callable is called as ``method(fun, args, **kwargs, **options)``
+ where ``kwargs`` corresponds to any other parameters passed to `minimize`
+ (such as `bracket`, `tol`, etc.), except the `options` dict, which has
+ its contents also passed as `method` parameters pair by pair. The metho
@dlax

dlax Feb 23, 2014

Member

metho

@dlax dlax commented on the diff Feb 23, 2014

scipy/optimize/_minimize.py
@@ -518,10 +644,14 @@ def minimize_scalar(fun, bracket=None, bounds=None, args=(),
warn("Method 'bounded' does not support relative tolerance in x; "
"defaulting to absolute tolerance.", RuntimeWarning)
options['xatol'] = tol
+ elif meth == '_custom':
+ options.setdefault('tol', tol)
@dlax

dlax Feb 23, 2014

Member

same remark as above

@dlax dlax and 1 other commented on an outdated diff Feb 23, 2014

scipy/optimize/tests/test_optimize.py
@@ -694,6 +732,37 @@ def test_minimize_scalar(self):
method='bounded').x
assert_allclose(x, self.solution, atol=1e-6)
+ def test_minimize_scalar_custom(self):
+ # This function comes from the documentation example.
+ def custmin(fun, bracket, args = (), maxfev = None, stepsize = 0.1,
+ maxiter = 100, callback = None, **options):
@dlax

dlax Feb 23, 2014

Member
  • align with parenthesis
  • no space around =
@pasky

pasky Feb 24, 2014

Contributor

Thanks, I'm fixing these issues also on the other lines pointed out. I learned something new - I didn't realize at all that PEP8 mandates e.g. no spaces around = in argument list!

@dlax dlax commented on an outdated diff Feb 23, 2014

scipy/optimize/tests/test_optimize.py
+ for testx in [bestx - stepsize, bestx + stepsize]:
+ testy = fun(testx, *args)
+ funcalls += 1
+ if testy < besty:
+ besty = testy
+ bestx = testx
+ improved = True
+ if callback is not None:
+ callback(x0)
+ if maxfev is not None and funcalls >= maxfev:
+ stop = True
+ break
+ return optimize.OptimizeResult(fun = besty, x = bestx,
+ nit = niter, nfev = funcalls, success = (niter > 1))
+ res = optimize.minimize_scalar(self.fun, bracket = (0, 4), method = custmin,
+ options = dict(stepsize = 0.05))
@dlax

dlax Feb 23, 2014

Member

same PEP8 remarks

@rgommers rgommers and 1 other commented on an outdated diff Feb 23, 2014

scipy/optimize/tests/test_optimize.py
@@ -433,6 +433,42 @@ def test_minimize_l_bfgs_b_ftol(self):
assert_allclose(v, self.func(self.solution), rtol=tol)
+ def test_custom(self):
+ """Custom minimizer"""
@rgommers

rgommers Feb 23, 2014

Owner

Can you remove the docstring? Tests should not have docstrings because those mess up nose verbose output; use a comment if necessary (not needed here).

@pasky

pasky Feb 24, 2014

Contributor

Will remove the docstring; I just imitated other nearby functions wrt. that.

@rgommers rgommers commented on an outdated diff Feb 23, 2014

scipy/optimize/tests/test_optimize.py
+ for s in [bestx[dim] - stepsize, bestx[dim] + stepsize]:
+ testx = np.copy(bestx)
+ testx[dim] = s
+ testy = fun(testx, *args)
+ funcalls += 1
+ if testy < besty:
+ besty = testy
+ bestx = testx
+ improved = True
+ if callback is not None:
+ callback(x0)
+ if maxfev is not None and funcalls >= maxfev:
+ stop = True
+ break
+ return optimize.OptimizeResult(fun = besty, x = bestx,
+ nit = niter, nfev = funcalls, success = (niter > 1))
@rgommers

rgommers Feb 23, 2014

Owner

Missing some blank lines in this code, especially here at the end of a function definition.

@rgommers rgommers commented on an outdated diff Feb 23, 2014

scipy/optimize/_minimize.py
@@ -315,7 +377,11 @@ def minimize(fun, x0, args=(), method=None, jac=None, hess=None,
else:
method = 'BFGS'
- meth = method.lower()
+ if isinstance(method, collections.Callable):
@rgommers

rgommers Feb 23, 2014

Owner

if callable(method): is better (here and above), and collections import can be removed.

@dlax dlax and 1 other commented on an outdated diff Feb 23, 2014

scipy/optimize/_minimize.py
+ ... besty = testy
+ ... bestx = testx
+ ... improved = True
+ ... if callback is not None:
+ ... callback(x0)
+ ... if maxfev is not None and funcalls >= maxfev:
+ ... stop = True
+ ... break
+ ... return OptimizeResult(fun = besty, x = bestx,
+ ... nit = niter, nfev = funcalls, success = (niter > 1))
+ >>> x0 = [1.35, 0.9, 0.8, 1.1, 1.2]
+ >>> res = minimize(rosen, x0, method = custmin,
+ ... options = dict(stepsize = 0.05))
+ >>> res.x
+ [ 1. 1. 1. 1. 1.]
+
@dlax

dlax Feb 23, 2014

Member

I'm not sure it's worth adding the example in the docstring. It's pretty long and I guess people wanting to use a custom method would figure out how to do it.

@pasky

pasky Feb 24, 2014

Contributor

I agree that the example is a bit long; not sure if it is a grave issue, but maybe there is some other good place to put it (that could be linked from the reference documentation)? I think providing an example as a basic template would be quite useful for anyone wishing to implement their own minimizer - it already trackes # of fevs and iterations, provides a forward-compatible calling convention, etc. I know that I'm personally always very glad for any examples of a functionality I can find in documentation.

Contributor

pasky commented Feb 24, 2014

Thank you both for reviewing the patch! I have updated the pull request with a revised commit, I believe (and hope) I have addressed all your comments (and I changed nothing else).

Coverage Status

Coverage remained the same when pulling a002360 on pasky:minimize-custom into 0da153e on scipy:master.

Member

dlax commented Feb 24, 2014

Petr Baudis a écrit :

Thank you both for reviewing the patch! I have updated the pull request
with a revised commit, I believe (and hope) I have addressed all your
comments (and I changed nothing else).

Looks good to me, except for the example in the docstring which I still
find a bit long and believe it could be removed. Anybody else has an
opinion on this?

Member

ev-br commented Feb 24, 2014

my 2cts: maybe a tutorial is a better home for this example

rgommers added this to the 0.14.0 milestone Feb 24, 2014

Owner

rgommers commented Feb 24, 2014

Tutorial makes sense. @pasky do you want to move that to doc/source/tutorial/optimize.rst?

Contributor

pasky commented Feb 25, 2014

All right, I have pushed an update that just moves the example code to a tutorial (with tiny introduction) and adds a reference to the tutorial to the docstring.

Coverage Status

Coverage remained the same when pulling e8d4678 on pasky:minimize-custom into 3b0c4a0 on scipy:master.

Member

dlax commented Feb 25, 2014

Just one more thing @pasky. Do you find it convenient to unpack the options dict in the custom method callable? The custom method could just be called as method(fun, x0, args=args, options=options, **kwargs). The point of having options unpacked for "built-in" solvers is to check if unused options are passed; that does not really make sense for a custom method I think.

Contributor

pasky commented Feb 25, 2014

I had two motivations to keep unpacking **options:

(i) Consistency. This seems to me to be a worthy goal by itself to me,
but it may also help in case a third-party method gets promoted to
scipy, the usage (of the method as well as of the parameters within the
method) will not have to change.

(ii) Convenience. If I accept options, I can simply specify them in
the parameter list of the function, including default values (or no
defaults if the option is required). Even in the example function, this
helps me avoid a lot of glue code I would have to use instead, instead
I can use a natural Python way to express which options I accept, which
are required and what the defaults are.

Contributor

pasky commented Feb 26, 2014

This update just adds a description of the change to the release notes.

@rgommers rgommers commented on an outdated diff Feb 26, 2014

scipy/optimize/tests/test_optimize.py
+ niter = 0
+ improved = True
+ stop = False
+
+ while improved and not stop and niter < maxiter:
+ improved = False
+ niter += 1
+ for testx in [bestx - stepsize, bestx + stepsize]:
+ testy = fun(testx, *args)
+ funcalls += 1
+ if testy < besty:
+ besty = testy
+ bestx = testx
+ improved = True
+ if callback is not None:
+ callback(x0)
@rgommers

rgommers Feb 26, 2014

Owner

TravisCI pyflakes run flagged that x0 is not defined here.

@rgommers rgommers commented on an outdated diff Feb 26, 2014

scipy/optimize/_minimize.py
@@ -66,6 +66,7 @@ def minimize(fun, x0, args=(), method=None, jac=None, hess=None,
- 'SLSQP'
- 'dogleg'
- 'trust-ncg'
+ - custom - a callable object
@rgommers

rgommers Feb 26, 2014

Owner

Finishing touch: can you add here in brackets that custom was added in 0.14.0

Owner

rgommers commented Feb 26, 2014

Other TravisCI failures are unrelated.

Owner

rgommers commented Feb 26, 2014

@dlax are you OK with the reply to your options question? If so I think this can go in after fixing the two trivial things I flagged.

Contributor

pasky commented Feb 26, 2014

Thanks, fixed the callback() parameter and added a note about version. (I think versionadded tag is not appropriate as that has to be a separate paragraph?)

Owner

rgommers commented Feb 26, 2014

Indeed, the way you added the version info looks good.

Member

dlax commented Feb 27, 2014

Ralf Gommers a écrit :

@dlax https://github.com/dlax are you OK with the reply to your
|options| question? If so I think this can go in after fixing the two
trivial things I flagged.

Yes, it's fine. Will merge.

@dlax dlax added a commit that referenced this pull request Feb 27, 2014

@dlax dlax Merge pull request #3369 from pasky/minimize-custom
ENH minimize, minimize_scalar: Add support for user-provided methods
3ec3cb6

@dlax dlax merged commit 3ec3cb6 into scipy:master Feb 27, 2014

1 check failed

default The Travis CI build could not complete due to an error
Details
Owner

rgommers commented Feb 27, 2014

Great, thanks @pasky and @dlax

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment