DOC: dual_annealing optimizer does not pass bounds to minimizer when minimizer_kwargs is partially given

[SterlingYM]

DOC: dual_annealing optimizer does not pass bounds to minimizer when minimizer_kwargs is partially given

[dschmitz89]

Thanks for the report @SterlingYM .

I understand that the API might be counter intuitive but passing bounds automatically to the local optimizer would break backwards compatibility. Another issue is that not all local optimizers support bounds.

I suggest that we add this behavior to the docstring explicitly. What do you think?

[SterlingYM]

@dschmitz89 I understand changing the implementation comes with some difficulties. At least I think adding docstrings is worth it, since I spent many hours debugging this issue and I am sure someone else would do that in the future.

A few alternative ideas:

1. Add a boolean option (something like pass_bounds=False) bounds are added to the user-defined kwargs when True. We can set it to False by default to maintain the backward compatibility.
2. Make additional option (minimizer_kwargs_extra={}) so that when this is used, these are strictly additional kwargs as the original docstrings suggest, not the only kwargs to be passed. If this is used only for updating the existing kwargs (whether user-defined or default), this is also backward compatible.

self.kwargs = minimizer_kwargs
self.kwargs_extra = minimizer_kwargs_extra
if not self.kwargs:
    # define kwargs, same as the original implementation, and add the following
    self.kwargs.update(self.kwargs_extra)
else:
    self.kwargs.update(self.kwargs_extra) # this is not necessary but just in case
# (or we can have the kwargs.updateI() line outside the if statement)

I am suggesting this because the current workaround to specify bounds twice in a single function call of dual_annealing seems silly and redundant, but again I understand if making such changes involves too much effort for this small issue. If that's the case, you can close the issue once the docstrings are changed.

[dschmitz89]

@dschmitz89 I understand changing the implementation comes with some difficulties. At least I think adding docstrings is worth it, since I spent many hours debugging this issue and I am sure someone else would do that in the future.

A few alternative ideas:

1. Add a boolean option (something like pass_bounds=False) bounds are added to the user-defined kwargs when True. We can set it to False by default to maintain the backward compatibility.
2. Make additional option (minimizer_kwargs_extra={}) so that when this is used, these are strictly additional kwargs as the original docstrings suggest, not the only kwargs to be passed. If this is used only for updating the existing kwargs (whether user-defined or default), this is also backward compatible.

self.kwargs = minimizer_kwargs
self.kwargs_extra = minimizer_kwargs_extra
if not self.kwargs:
    # define kwargs, same as the original implementation, and add the following
    self.kwargs.update(self.kwargs_extra)
else:
    self.kwargs.update(self.kwargs_extra) # this is not necessary but just in case
# (or we can have the kwargs.updateI() line outside the if statement)

I am suggesting this because the current workaround to specify bounds twice in a single function call of dual_annealing seems silly and redundant, but again I understand if making such changes involves too much effort for this small issue. If that's the case, you can close the issue once the docstrings are changed.

I fully agree that the current API seems silly. In fact, scipy.optimize is not exactly an example of state of the art API design (3 different ways to specify bounds, arbitrary strings instead of enums just to name two). I have spent my fair share of time debugging API related issues as well .. class based optimizers would probably be the way improve things in the future but realistically, I do not see it happening. scipy.optimize is used a LOT so downstream users/libraries will likely resist changes.

Adding one more option makes this already complex API even harder to grasp though in my opinion. Slightly related question: did you encounter the same issue with optimize.shgo by any chance? It uses a similar API from what I know. Whatever we decide to do, it should be applied to shgo as well.