WIP: Basin hopping improvements

[endolith]

Based on #7799

• Improve documentation
• Keep local minima even if step is rejected?
  • This will never happen with the default Metropolis, but could happen with a custom accept_test
  • Not sure, save for another PR
• Don't keep local minima if minimization fails result is invalid
  • But the step location should still be kept in that case?
    • If it's just out of bounds it's ok to keep, but maybe a bad idea.
    • If it's numerically wrong then it shouldn't be kept. Not sure if this is possible.
• Should be possible to specify stepsize of same shape as starting guess (when variables have different units/scales)
• Support Monotonic Basin-Hopping (T=0)
  • Fixed in ENH: Basin-hopping changes #7954 and TST, DOC: Basin-hopping changes attempt 2 #8050
• Support Multistart? (or just explain how to configure it (reject all steps and make stepsize wide enough to cover area of interest))
  • Should just be a different function, with a more clear interface and parallelization: Adding a multistart algorithm to global optimizers in scipy.optimize #6381
• First minimization should behave identically to subsequent ones? (reject InvalidResult, etc.)
  • This requires initializing with a dummy result with function value +infinity so that subsequent results replace it.
• Write tests for new stuff
• Add InvalidResult test to brute() (separate PR)

[endolith]

Ok so here's a nice graphic of how the algorithm works from doi:10.1155/2012/674832:

SciPy's behaves the same way, always accepting hops to lower energies (C2) and sometimes (randomly) accepting hops to higher energies (C3), in the hope that there will be another better minimum further along in that direction. Otherwise it rejects the step (C4) and goes back to previous location (C3) to make the next step.

This is unrelated to the failed minimization issue:

It's conceivable that someone could change accept_test to sometimes reject lower minima, too, in which case the lower minimum won't be stored as a global minimum candidate. But accept_test is about rejecting the step, not the result, so I think it should always store a minimum even if it doesn't step further in that direction.

So

        if accept:
            self.energy = minres.fun
            self.x = np.copy(minres.x)
            new_global_min = self.storage.update(minres)

should become

        if accept:
            self.x = np.copy(minres.x)
        self.energy = minres.fun
        new_global_min = self.storage.update(minres)

?

[rgommers]

That graphic is quite nice for understanding. C3 should be colored differently I think, because it's not always accepted. But other than that I'd be +1 on adding an illustration like that to the docstring.

[endolith]

If one of the customized accept_test functions returns None, should that raise an Exception? Currently it's treated as False, but it probably means "the accept test is broken" rather than "the accept test rejected the step". (I did this by accident using the test to plot points.)

[andyfaff]

In Python `bool(None) is False`

…

On 3 September 2017 at 09:44, endolith ***@***.***> wrote: If one of the customized accept_test functions returns None, should that raise an Exception? Currently it's treated as False, but it probably means "the accept test is broken" rather than "the accept test rejected the step". (I did this by accident using the test to plot points.) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#7819 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAq51njOOtRXID_aCyzUalsszvIqVwqiks5seehUgaJpZM4PKyZC> .

--

_____________________________________ Dr. Andrew Nelson

_____________________________________

[endolith]

@andyfaff Yes, and that's why it's happening, but I think if accept_tests returns None, then something is wrong with accept_tests and it should be caught.

[endolith]

Ok, so I made some preliminary changes, described in the commit messages. Basically:

• Don't update the global minimum if minimization failed
• Do update global minimum even if step was rejected (in case of a custom random accept_test, for instance)

There are several variables with similar names and it's kind of confusing which is which, so I made a list:

• self.x
  • starting point for next step (last minimum accepted)
  • then fed to accept_test and step-taking report as x_old
  • if step (last minimum vs current minimum) is acceptable, this gets updated to current minimum
• self.energy
  • energy of starting point (last minimum accepted)
  • printed in step summary
  • fed to accept_test and step_taking report as f_old
  • updated to last minimum but only if step is accepted
  • also used to print global minimum replaced with storage.minres.fun
  • also used to print current local minimized value (f)
• x_after_step
  • position after taking a random step
  • used as starting point for local minimization
• x_after_quench
  • position after taking a random step and then local minimization
  • fed to accept_test and step_taking report as x_new
• energy_after_quench
  • function value after taking a random step and then local min
  • fed to accept test and report as f_new
• storage.minres.x and storage.minres.fun
  • best global minimum candidate found
  • only updated if minimization succeeds

Please check if I'm getting this right.

Originally I thought that the initial minimization should be part of the main loop and identical to the others, but I guess that's not right. If we think of the "step" as the random jump in starting point, the algorithm works like this:

• starting guess -> local minimum (0 steps)
• randomized step from local minimum to new starting guess
• starting guess -> local minimum (1 steps)
• randomized step from local minimum to new starting guess
• starting guess -> local minimum (2 steps)

So niter is the number of steps, and the number of minimizations is always niter+1.

So I changed i so that if you say niter=0, it does 1 minimization and 0 steps and then returns result.nit=0. (Previously it said nit=1 for both niter=0 and niter=1.) But is that right? nit is described as "Number of iterations performed by the optimizer." Iterations of the hopping, or iterations of the local minimization?

Anyway, it seems to work ok with constraints now:

(eggholder function, blue is starting guess, green are random jump locations, yellow are local minima, and red is global minima, red regions are out of bounds)

[juliusbierk]

Why this change? If the function, for any reason, is smaller than the current global minimum, it's a better minimum, isn't it? It might not be the global minimum if the minimisation failed, but it's certainly better than a local minimum with a higher function value?

[juliusbierk]

Especially if a minimiser is being used with a small nfev, many minimisations will fail, but basinhopping can still be smart choice for finding approximations to the global minimum. I would argue that it should at least be an option to save as global minimum even if minimisation failed.
I agree that this shouldn't be stored as a local minimum, but it's still the best bet for the global minimum, right?

[endolith]

@juliusbierk The reason I started making these changes was that it was keeping results even when minimization failed, leading to results that violated the given constraints: #7799

For other types of minimization failure, can we even trust that the value is really correct for the function?

[juliusbierk]

@endolith Ah! I see. Yes, with bounds I understand why this is a problem. I see other use-cases, however, where the former behaviour is preferred (for my use cases, that's typically the case). Could this simply be made an option? Or perhaps bounds should be passed directly to basinhopping? Not sure at all what the best approach, and maybe I'm fairly alone with the preference for the former behaviour.

The values that minimizers return will always be the result of a function evaluation, won't it? At least with the minimizers I know of.

[endolith]

@juliusbierk Well I thought of passing bounds or constraints to basinhopping itself, but it still needs to pass them to the local minimizer, and it would also need some kind of tolerance argument too, or it will reject a lot of things because of floating point inequality, like the local minimizer will find 49.99999998 as meeting a constraint of >=50, which the basinhopping would then reject.

The values that minimizers return will always be the result of a function evaluation, won't it?

I don't know. When it fails for things like "singular matrix" it just means it can't find where to go, but the result given is from a single call to the function given, so it's still a valid result, just not guaranteed to be a real minimum?

[juliusbierk]

I see. Yes, that would make it a lot more tedious.

I agree with you. It won't be a real minimum, but it is the "lowest value found", and I would always expect the lowest value found to be returned rather the "the lowest minimum found".

[endolith]

Yes I agree that it should keep "the lowest value found" as long as it meets the bounds and constraints, and is the result of an evaluation of the original function, even if the minimization fails. But how do we do that, since going out of constraints causes a minimization failure, too?

[endolith]

Originally I thought that the initial minimization should be part of the main loop and identical to the others

Originally I thought this could be easily adapted to Multistart by rejecting all steps and setting the stepsize wide enough to cover the desired area (like to uniformly sample -5 to +15, set starting guess at +5 and stepsize to 10: basinhopping(func, 5, stepsize=10, accept_test=lambda x: False)

But because it does a local minimization even before the first step, this won't work because it moves the starting point to that minimum first.

[endolith]

So we need to:

• keep the result of local minimization if "minimization failed" but the result is still within bounds and constraints and the result of a valid function call
• reject the result of local minimization if "minimization failed" due to going out of bounds or constraints

Any ideas how to do this?

[juliusbierk]

@endolith I don't know how consistent the status flag is in the OptimizeResult that results from minimize, but one method would be to use these flags if one of the contraints/bounds methods are being used, ala

if res.method in [' L-BFGS-B', 'TNC', ...] and res.status < 2

[endolith]

Yes I thought of using that, but not sure if it's reliable enough, and doesn't each solver have different sets of values? and those could change? especially if new solvers are implemented?

Maybe the real solution is to change success to non-binary:

• True = valid local minimization
• False = valid function value, but not the local minimum
• None = invalid result

Or it could return something more explicit, like FailedMinimization of FailedMinimizationType, the way NotImplemented is returned by comparison methods.

Actually it probably has the be the latter, since bool(None) == bool(False).

[endolith]

mode	message	success	proposed
SLSQP
-1	Gradient evaluation required (g & a)	N/A	N/A
0	Optimization terminated successfully.	True	True
1	Function evaluation required (f & c)	N/A	N/A
2	More equality constraints than independent variables	False	???
3	More than 3*n iterations in LSQ subproblem	False	False
4	Inequality constraints incompatible	False	Invalid
5	Singular matrix E in LSQ subproblem	False	False
6	Singular matrix C in LSQ subproblem	False	False
7	Rank-deficient equality constraint subproblem HFTI	False	False
8	Positive directional derivative for linesearch	False	Invalid
9	Iteration limit exceeded	False	False
COBYLA
1	Optimization terminated successfully.	True	True
2	Maximum number of function evaluations has been exceeded.	False	False
3	Rounding errors are becoming damaging in COBYLA subroutine.	False	???
4	Did not converge to a solution satisfying the constraints.	False	Invalid
TNC
-1	Infeasible (lower bound > upper bound)	False	Invalid
0	Local minimum reached (|pg| ~= 0)	True	True
1	Converged (|f_n-f_(n-1)| ~= 0)	True	True
2	Converged (|x_n-x_(n-1)| ~= 0)	True	True
3	Max. number of function evaluations reached	False	False
4	Linear search failed	False	???
5	All lower bounds are equal to the upper bounds	False	Invalid?
6	Unable to progress	False	???
7	User requested end of minimization	False	False
L-BFGS-B
0	converged	True	True
1	too many function evaluations or too many iterations	False	False
2	if stopped for another reason, given in d['task']	False	???

[juliusbierk]

@endolith I personally like this approach of changing minimize. It's very nice that only algorithms that can return non-sensical values have to be changed. Nelder-Mead, for instance, can simply keep returning True and False...
None seems a bit arbitrary, perhaps I agree that FailedMinimization would be more clear. However, I think it's good if bool(FailedMinimization) == bool(False), since that would help with backwards compatibility...

[endolith]

@juliusbierk Do you have an opinion on the question marks in my table?

I think it's good if bool(FailedMinimization) == bool(False), since that would help with backwards compatibility...

Good point.

[juliusbierk]

@endolith Sorry, I definitely don't know off-hand. I had a quick look at the TNC code. There are also -3 and -2: https://github.com/scipy/scipy/blob/master/scipy/optimize/tnc/tnc.h#L68, which are FailedMinimization.

4: TNC_LSFAIL looks like it's just not finding a minimum: https://github.com/scipy/scipy/blob/master/scipy/optimize/tnc/tnc.c#L1628, so this, I think, should be False.

6: NOPROGRESS looks like it's stuck somewhere: https://github.com/scipy/scipy/blob/master/scipy/optimize/tnc/tnc.c#L791 so this suggests False, but I'm not sure if all constraints/bounds are guaranteed to be satisfied.

I don't use these constrained minimizers much, so I'm really no expert. Hopefully someone else can pitch in.

[endolith]

@juliusbierk

I see other use-cases, however, where the former behaviour is preferred (for my use cases, that's typically the case).

Do you have any simple examples of your use case, where the local minimizer fails to converge but the result is still useful?

[endolith]

Ok I started over, implementing the InvalidResult flag. Please comment on whether I should keep going with this.

[endolith]

It works now, bounds and constraints are both respected, fixing #7842:

[chernals]

Has a consensus been found on this? Any chance this could be merged soon?

[endolith]

@chernals I'm waiting for comments by others on whether this is a good idea.

[JorgenPhi]

The PR here really helped bring my optimization a lot closer to the result. However, I ended up checking only for minres.success == True instead of InvalidResult, but this could just mean that InvalidResult needs additional cases triggered.

Doing this lead my basinhopping optimization to much more reasonable results compared to what the current 1.4.x branch generates.

Before this PR basinhopping would just get further and further away from satisfying the constraints, and focused its attention solely on minimizing.

[JorgenPhi]

Following up on this, because I'm sick of my docker instances taking forever to build scipy instead of using a wheel 😂

Anyways, I guess I fail to see why we store the energy, x, and update the "global_min" on an optimization trial that did not result in minres.success?

This line in particular https://github.com/scipy/scipy/blob/maintenance/1.5.x/scipy/optimize/_basinhopping.py#L154

Thanks in advance.

[endolith]

So it's been 3 years since I was working on this, and I barely remember what I was doing. It needs rebasing, since changes have been made since.

Has anyone read through it lately and can comment on whether this is a good approach or not? Such as the InvalidResult object, etc.

Can anyone help fill in missing values in the table of return values?

It primarily needs technical input from other people.

[mdhaber]

basinhopping did not perform very well in a recent run of the global optimization benchmarks. It sounds like there is support for this PR, though - might it improve the performance?

If you're interested in finishing this up @endolith, I can review it. Please let me know.
If not, I'll go ahead and close, and it can be re-opened if someone is interested in reviving it.

[endolith]

basinhopping did not perform very well in a recent run of the global optimization benchmarks. It sounds like there is support for this PR, though - might it improve the performance?

"Performance" = "likelihood of finding the correct answer", right? Or "computational speed"?

I don't know if it would improve it. It's been 5 years since I made this, but if I remember correctly, it would only improve the likelihood of finding the solution when there are bounds or constraints. If there aren't any, it should have no effect. Do the benchmarks use those?

[endolith]

This will still print accept 1 when it hits self.print_report(minres.fun, accept), even though the step is not "accepted". Is that good or bad?

Adding the success criteria to the accept_tests isn't possible because those are based only on the new and old values.

[mdhaber]

Performance = % of successes and number of objective function evaluations. All four other global optimizers tested did better in both categories. I remember adding some problems for global optimizers with constraints... But those may have been tests, not benchmarks. I think that few, if any, of the benchmarks have constraints.

[endolith]

Reading through all this, the original problem could be solved simply by adding something like

            elif not minres.success:
                accept = False
                break

to the for test in self.accept_tests loop. Then steps would not be "accepted" if they went out of bounds. It seems that the original authors wanted those kept, though, since they explicitly added a warning for them but didn't reject them? (Which was the motivation for me creating the InvalidResult thing, to keep them as intermediate steps but not return them as solutions, since they might help find solutions hidden behind constraints.) But that would be the quick obvious fix for returning wrong values.

But seeing how the global optimizers add different keys to their output, I think adding more keys to OptimizeResult might be a better solution than creating an InvalidResult class. Currently OptimizeResult says whether the minimization failed or succeeded, and includes the specific numeric error codes for failure, but those mean nothing to basinhopping. The number codes could be interpreted into another key that indicates whether the result is out of bounds or not?

This algorithm is similar to Dual Annealing, which seems to reject out of bounds values after the local minimization step:

https://github.com/scipy/scipy/blob/v1.9.1/scipy/optimize/_dual_annealing.py#L425

scipy/scipy/optimize/_dual_annealing.py

Lines 423 to 430 in 2e5883e

	# Check if is valid value
	is_finite = np.all(np.isfinite(mres.x)) and np.isfinite(mres.fun)
	in_bounds = np.all(mres.x >= self.lower) and np.all(
	mres.x <= self.upper)
	is_valid = is_finite and in_bounds
	# Use the new point only if it is valid and return a better results
	if is_valid and mres.fun < e:

[JorgenPhi]

The minres.success check was the exact approach I had used when we were utilizing a basinhopping minimization with constraints. Before that modification, the procedure would take drastically longer and would fail to find a solution that would satisfy the constraints.

[mdhaber]

@endolith looks like that link to the "original problem" in your post is broken.

It sounds like you want to go a different direction here, and merging main here is not necessary?

[endolith]

@mdhaber

It sounds like you want to go a different direction here, and merging main here is not necessary?

Yeah, there was no consensus, so I broke off #7954 from this, for example, and other things can be broken off into smaller PRs, too.

1. Reject out of bounds with a heavy-handed elif not minres.success to quickly fix the actual issue. Not sure if this is the right place to put this, but probably. It can't be in the list of accept_tests since those are based only on Before and After values and don't see the OptimizeResult.success.
2. Enhance it by adding some kind of key to the OptimizeResult from local minimizers that means "this is a valid function value, but out of bounds, which you can use as a step if you want"
3. Add the key to each local minimizer (maybe one PR for each), after decoding what their return code numbers actually mean

[mdhaber]

OK @endolith but please note that #1 of #7799 (comment) is distinct from #1 of this list #7819 (comment) (as are the rest of these numbered items). Both #1s are simple fixes of the bug, but I would recommend #1 of #7799 (comment); #1 here is somewhat contentious and I would not be interested in reviewing that, personally.

[mdhaber]

OK, based on #7819 (comment) I'll close this.