BUG: minimize: fix for powell method #21092
Conversation
|
This PR doesn't seem to make a lot of sense and is a little suspicious. |
|
Can you elaborate on what doesn't make a lot of sense to you? |
Please forgive me, I was hasty to judge. I was confused because The following example on the current main (i.e. without the fix applied) reveals no circumstances where import numpy as np
from scipy.optimize import minimize, rosen
class CallbackMinimize:
def __init__(self):
self.f_vals = []
self.x_vals = []
def __call__(self, intermediate_result):
self.f_vals.append(intermediate_result.fun)
self.x_vals.append(intermediate_result.x)
def test_func(x):
return np.sum(x ** 2)
call_fun = CallbackMinimize()
x_min, x_max = 0, 10
bnds = [(x_min, x_max)]
res = minimize(
test_func,
x0=[9],
bounds=bnds,
method='Powell',
tol=1e-6,
callback=call_fun
)
for i in range(len(call_fun.f_vals)):
np.testing.assert_equal(call_fun.f_vals[i], test_func(call_fun.x_vals[i]))
np.testing.assert_equal(test_func(res.x), res.fun)
|
|
Sorry for the confusion! I just wanted to give context about where my problem arose, which was because I minimised a lot of functions (that stemmed from a large population I generated with Concerning the unit test you showed, the problem is not that As far as I see, all of this only becomes a real problem in the following case: starting with some initial I know this is a special case, but when I minimised a large amount of functions, this I hope this clears everything up a bit. |
I think I can just about follow the logic in your post, but it's not easy. I think it's ok to introduce the |
You're right, that's what I meant to say. Originally I wasn't even planning on changing the entries of From my side it would be okay to take out the unit test, leave |
|
@andyfaff :) |
|
Since I couldn't come up with a better unit test for the behaviour, I deleted the unit test and reverted the corresponding change in OptimizeResult. |
|
@andyfaff @irideselby where does this stand? we're likely a few days from branching for |
|
I've lost currency with the changeset here. I don't think I'll be able to revisit before branching |
|
Alright, I'll bump this one for now--this is definitely a strange MR with 1-line change! |
|
I went through the comments in this PR again today because I try to avoid blindly bumping PRs just before a release, especially for first time contributors. I sat down with @j-bowhay for a little bit and I think we agreed that we're just not comfortable enough on the justification for the change here to merge at this time. It seems a bit tricky to justify without a robust reproducer that could be used as a unit test. I'm not trying to discourage pursuing the fix though, and it does sounds like the developer here really is trying to communicate a complex problem, but it is still a bit hard for me to understand at this time. So, I will bump the milestone again--let us know what we can do to help. I think a simple reproducer/unit test is the clearest route to getting this across the line. |
Hello all,
I encountered an issue in the minimize function while working with differential evolution. For some members of the population, the direc1 value of the extrapolated point became zero without being caught by the check in
_linesearch_powell, meaning the optimisation could not progress further.It turns out this problem arises during the iteration of successive points in
_minimize_powell: In each step, the current point is stored for comparison with the subsequent point, from which the direc1 value for the next iteration is calculated. While the f(x)-value of each point is correctly stored, the corresponding x-value is not stored at all if the condition fx > fx2 is fulfilled. That means the current point will be stored with the correct f(x)-value but with the wrong (previous) x-value.In some circumstances it can now happen that in one iteration
_linesearch_powellresults in a point with2.0 * (fx - fval) > bnd, meaning the function doesn't break out, but withx - x1 = direc1 = 0, meaning the optimisation cannot progress further.This pull request fixes the storing of x-values, such that all points are stored correctly for comparison with subsequent points of the iteration. Additionally, a unit test is implemented to check the correct behaviour.