MAINT: Add graceful handling of invalid initial brackets to elementwise bracket finding functions #20685
Conversation
github-actions
bot
added
scipy.optimize
scipy._lib
maintenance
For the sake of simplicity (future maintenance, array API conversion) I've wondered whether we should restrict |
scipy/optimize/_bracket.py
Outdated
| @@ -246,11 +255,13 @@ def post_func_eval(x, f, work): | |||
|
|
|||
| def check_termination(work): | |||
| stop = np.zeros_like(work.x, dtype=bool) | |||
| # Condition 0: initial bracket is invalid | |||
| stop[work.status == eim._EINPUTERR] = True | |||
There was a problem hiding this comment.
A special case that I don't really want to deal with is xl0 == xr0 == true_root : ( Maybe we should just add a note about it?
There was a problem hiding this comment.
I haven't addressed this yet. I'd rather not deal with that case either. It looks like the documentation currently doesn't even mention that xmin <= xl0 < xm0 < xr0 <= xmax is needed. I think just adding a note about that should be enough; the behavior will be exactly as documented.
There was a problem hiding this comment.
I just put things like this
- ``-5`` : The initial bracket does not satisfy
`xmin <= xl0 < xm0 < xr0 <= xmax`.
when documenting the error codes. Do you think that's sufficient, or should this go somewhere else as well? I was thinking of putting it in the Parameters documentation, but wasn't sure where to include it, since it pertains to every one of these bracket parameters. I guess it could go in the xm0 section, stating that the parameters are defined below; or in the xmin, xmax section.
scipy/optimize/tests/test_bracket.py
Outdated
| lambda x: x, | ||
| lambda x: x] |
There was a problem hiding this comment.
I think we only need to have one example of each status code here; I'd prefer that rather than having one example of the first few code and many examples of the last code. My intent when writing this test was to show that the codes could all be different; it really is elementwise. There could be a separate function to test the logic for generating this status code with many different cases, if desired (i.e. all possible invalid orderings). But maybe the most natural example here would be to have everything in reverse order, xmin > xl0 > xr0 > xmax.
Co-authored-by: Matt Haberland <[email protected]>
|
Thanks @steppi! |
Reference issue
What does this implement/fix?
This PR updates the private
_bracket_minimumand_bracket_rootfunctions fromscipy.optimize._bracketto gracefully handle cases where the initial bracket is invalid. Previously aValueErrorwas raised if any invalid initial brackets appear among the set of input initial brackets (these are vectorized functions which can takes arrays for bracket endpoints and other parameters). These functions have been updated to terminate in the cases where the initial bracket is invalid with an exit status indicating a failure due to an invalid initial bracket. All other cases proceed as usual.Additional information
In
scipy.optimize.tests.test_bracket, I moved all of the relevant test cases from thetest_input_validationtest methods into the correspondingtest_flagstest methods. This might be overkill, so I'm happy to remove some of these if you think that's fine @mdhaber; right now a majority oftest_flagstest cases are due to invalid initial brackets.I called the error code
_EINPUTERR, but would be open to suggestions. I didn't put much thought into this name. The way I implemented this is pretty clean for_bracket_minimumbut gets a little convoluted with_bracket_rootdue to having to work with trickery involved to get the left and right searches to proceed together.