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

[SterlingYM]

Issue with current documentation:

Regarding the documentation of dual_annealing:

The current implementation of dual_annealing only generates the new kwargs dict for the local minimizer when there is NO minimizer_kwargs provided.
https://github.com/scipy/scipy/blob/4ee0de31b758f81be85435a0ad72d315a840441d/scipy/optimize/_dual_annealing.py#L403C1-L413C70

This causes issues during local search, such as below:

dual_annealing(
    func,
    x0 = x0,
    args = args,
    bounds = bounds,
    minimizer_kwargs = dict(options=dict(eps=1e-5))
)

The only kwargs items passed to the minimizer is {'eps': 1e-5}, where users like myself might expect {'eps': 1e-5, 'bounds': bounds} to be passed.
This results in an extremely inefficient local search, since the local minimizer finds some coordinates outside the bounds only to get rejected by the sanity check code (below).
https://github.com/scipy/scipy/blob/4ee0de31b758f81be85435a0ad72d315a840441d/scipy/optimize/_dual_annealing.py#L428C1-L431C43

To avoid this, current workaround is to add bounds to minimizer_kwargs:

dual_annealing(
    func,
    x0 = x0,
    args = args,
    bounds = bounds,
    minimizer_kwargs = dict(bounds=bounds, options=dict(eps=1e-5))
)

Idea or request for content:

It will be a great help for users to explicitly mention in the document (or change the implementation so that default kwargs get updated by user kwargs: see below) that minimizer_kwargs will be the ONLY kwargs that get passed to the minimizer.
Current description of minimizer_kwargs option is

Extra keyword arguments to be passed to the local minimizer

and this is misleading, since it gives an impression that only the modification to the default values (such as eps parameter in the example above) are needed, in which case users may think bounds is not necessary.

---

Alternatively, changing the implementation (of the kwargs generator code I linked above) to something like this

minimizer_kwargs_default = {
    'method': 'L-BFGS-B',
    'maxiter': ls_max_iter,
    'bounds': list(zip(self.lower, self.upper))
}

minimizer_kwargs_updated = minimizer_kwargs_default.copy()
if self.kwargs:
    minimizer_kwargs_updated.update(self.kwargs)

might match the expected behavior.

Additional context (e.g. screenshots, GIFs)

No response

[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.