Tweaks to minimize() behavior, for discussion #217

pv · 2012-05-19T12:28:55Z

Here's a bunch of tweaks to the minimize() behavior:

Raise a warning when encountering options unknown to the solver.
Add a tol parameter to minimize*() functions, for easy tolerance specification
In solvers, rename maxfev to maxiter if the solver does not have a maxiter parameter. This makes the claim in minimize() that maxiter is a common parameter for all solvers true. Iterations are in any case fuzzily defined, so we may as well make this change...
Rename pgtol to gtol
In Cobyla, rename rhoend to tol. It cannot probably be called xtol although it is an x-tolerance, as it seems to be an absolute tolerance rather than a relative one.

Internal reorganization:

Use Python parameter unpacking for extracting parameters from the options dict. The internal minimize* routines now specify any extra options they accept in their argument list, as usual. This is probably slightly cleaner and faster (didn't benchmark) way to unpack the parameters. It also allows easy detection of unknown parameters, without needing to modify the input dict (which makes the .pop() approach require an copy of the dictionary).

And some bugfixes:

ftol in brent and golden is actually xtol
There were a couple of unknown options passed down to the solvers, which was caught by the added warnings. I think checking the options dict for unknown keys is a must --- IIRC, this is something that I disliked also in Matlab's optimization interface.

pv · 2012-05-19T12:29:52Z

Note that some of the above changes can be argued against ... while the changes above make sense for me, it's not clear where the optimum exactly lies.

dlax · 2012-05-21T14:40:23Z

I agree with most of the changes you propose. Catching unknown options is a very good thing in particular.

Concerning the new tol parameter, I think the idea of solver-agnostic tolerance specification a good idea but I am not sure it should be a distinct keyword parameter. Maybe this tol parameter could simply replace existing *tol parameters in options. This would just lead to one more generic parameter in options.

dlax · 2012-05-21T15:22:20Z

Also, there is a few warnings in tests. Did you keep those on purpose?

pv · 2012-05-21T15:28:04Z

21.05.2012 17:22, Denis Laxalde kirjoitti:

Also, there is a few warnings in tests. Did you keep those on purpose?

Nope. Those should probably be fixed, too.

dlax · 2012-05-21T15:33:23Z

Pauli Virtanen a écrit :

Also, there is a few warnings in tests. Did you keep those on purpose?

Nope. Those should probably be fixed, too.

Ok. See 26d6d9fccaabd267182990ba80a13818c318f530 in my minimize-opts branch.

rgommers · 2012-06-04T17:14:09Z

The only point to resolve here seems to be the tol parameter. I don't really see how it can replace (one of) the tolerance parameters in options, since there are a number of different ones and there isn't one that's much more common than the others.

The approach of Pauli should give reasonable behavior as long as the scales of both x and f(x) are of order 1. If f(x) is O(1) and x is very small, then the difference between ftol and gtol makes it hard to find a value that works universally.

Whether it's desirable to then add a tol parameter I'm not sure about.

dlax · 2012-06-05T15:10:33Z

Now that PR #195 is merged, this one will need a bit of work in order to be merged.
In particular:

show_minimize_options function has been replaced by show_options and has been moved from _minimize.py to optimize.py
root and underlying solvers would have to be updated to follow the internal reorganization.

I can spend some time on this but we should coordinate @pv.

rgommers · 2012-06-09T13:49:29Z

Who has time to finish this PR this weekend? I think this should land in 0.11, because now we can still make changes without having to worry about backwards compatibility.

pv · 2012-06-09T14:59:35Z

I have some time. As far as I see, apart from the last commit (addition of tol) this would be mostly ready...

rgommers · 2012-06-09T15:02:52Z

Looks like it. The two bullets from Denis above need addressing though, and silencing/fixing those warnings.

Would be nice to get tol in too if possible.

…nimize_* This simplifies, and probably speeds up the argument unpacking code. Also, check for unknown options, and raise warnings if they are present.

…nce is x-tolerance

Caught by the new unknown options warnings.

Rename option solver parameters: - Add `ftol` to lbfgsb and remove `factr`. - Rename `pgtol` to `gtol` everywhere. - Rename `maxfev` to `maxiter` if the solver did not already have `maxiter`. The aim here is to keep the options as similar as possible across different solvers, preferring to allow slight changes in meaning rather than chaning the option name.

The name `rhoend` is rather confusing for what essentially is a tolerance.

…nused options Fix also unused option warnings in tests.

The plain `ftol` and `xtol` names are used in other routines for relative error, so better keep the nonlin.py solvers in line.

pv · 2012-06-09T17:11:39Z

Rebased on top of master, and did the corresponding changes also in root().

dlax · 2012-06-09T17:55:03Z

scipy/optimize/optimize.py

@@ -18,10 +18,12 @@
 __all__ = ['fmin', 'fmin_powell', 'fmin_bfgs', 'fmin_ncg', 'fmin_cg',
           'fminbound', 'brent', 'golden', 'bracket', 'rosen', 'rosen_der',
           'rosen_hess', 'rosen_hess_prod', 'brute', 'approx_fprime',
-           'line_search', 'check_grad', 'Result', 'show_options']
+           'line_search', 'check_grad', 'Result', 'show_options',
+           'OptimizeWarning']


Out of curiosity, why is OptimizeWarning in __all__?

It (and other custom Warning classes) are in __all__ because people may want to silence them via warnings.simplefilter("ignore", category=OptimizeWarning). Then, it is useful to be able to import it from the public namespace.

dlax · 2012-06-09T18:35:47Z

Pauli Virtanen a écrit :

Rebased on top of master, and did the corresponding changes also in root().

It looks good. I'll do a final check and merge afterwards.
Thanks.

Tweaks to minimize(), minimize_scalar() and root() behavior.

pv and others added 11 commits June 9, 2012 17:55

ENH: optimize: move options dictionaries to function arguments in _mi…

d659eb7

…nimize_* This simplifies, and probably speeds up the argument unpacking code. Also, check for unknown options, and raise warnings if they are present.

TST: optimize: add a test for lbfgsb ftol option

8b359ff

BUG: optimize: rename ftol -> xtol in brent and golden --- the tolera…

731e7df

…nce is x-tolerance

BUG: optimize: fix unsupported solver option names

883c467

Caught by the new unknown options warnings.

ENH: optimize: rename rhoend -> tol in cobyla

535b17c

The name `rhoend` is rather confusing for what essentially is a tolerance.

ENH: optimize: add a convenience tol= argument to minimize*()

a453821

TST: fix warnings for unknown optimize options, and jac

7c191ce

ENH: optimize: reorganize root() internal option passing, and catch u…

1579089

…nused options Fix also unused option warnings in tests.

ENH: optimize: rename x/ftol,x/frtol->x/fatol,x/ftol to be consistent

51511ac

The plain `ftol` and `xtol` names are used in other routines for relative error, so better keep the nonlin.py solvers in line.

ENH: optimize: add tol= option to root()

c323667

dlax reviewed Jun 9, 2012
View reviewed changes

dlax added a commit that referenced this pull request Jun 9, 2012

Merge pull request #217 from pv/minimize-opts

060f78e

Tweaks to minimize(), minimize_scalar() and root() behavior.

dlax merged commit 060f78e into scipy:master Jun 9, 2012

argriffing mentioned this pull request Jan 2, 2015

DOC: indicate possibility that line_search returns some values that are None #4312

Merged

AtsushiSakai mentioned this pull request Apr 6, 2024

DOC: optimize.root(method='lm') option #20392

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tweaks to minimize() behavior, for discussion #217

Tweaks to minimize() behavior, for discussion #217

pv commented May 19, 2012

pv commented May 19, 2012

dlax commented May 21, 2012

dlax commented May 21, 2012

pv commented May 21, 2012

dlax commented May 21, 2012

rgommers commented Jun 4, 2012

dlax commented Jun 5, 2012

rgommers commented Jun 9, 2012

pv commented Jun 9, 2012

rgommers commented Jun 9, 2012

pv commented Jun 9, 2012

dlax Jun 9, 2012

pv Jun 9, 2012

dlax commented Jun 9, 2012

Tweaks to minimize() behavior, for discussion #217

Tweaks to minimize() behavior, for discussion #217

Conversation

pv commented May 19, 2012

pv commented May 19, 2012

dlax commented May 21, 2012

dlax commented May 21, 2012

pv commented May 21, 2012

dlax commented May 21, 2012

rgommers commented Jun 4, 2012

dlax commented Jun 5, 2012

rgommers commented Jun 9, 2012

pv commented Jun 9, 2012

rgommers commented Jun 9, 2012

pv commented Jun 9, 2012

dlax Jun 9, 2012

Choose a reason for hiding this comment

pv Jun 9, 2012

Choose a reason for hiding this comment

dlax commented Jun 9, 2012