Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MAINT: optimize.minimize: revert gh-11712 to fix macos_arm64_test #17308

Merged
merged 3 commits into from
Nov 6, 2022

Conversation

mdhaber
Copy link
Contributor

@mdhaber mdhaber commented Oct 30, 2022

Reference issue

gh-11649
gh-11712

What does this implement/fix?

macos_arm64_test has a failure in main:

FAILED scipy/optimize/tests/test_minimize_constrained.py::test_gh11649 - UserWarning: delta_grad == 0.0. Check if the approximated function is linear. If the function is linear better results can be obtained by defining the Hessian as zero instead of using quasi-Newton approximations.

This was just a warning, but it exposed a bug introduced in gh-11712. This PR reverts gh-11712 but leaves the test (and strengthens it), xfailing it with strict=True. The original issue, gh-11649, has been re-opened, and the xfail mark can be removed when it is resolved.

Additional information

@dpoerio I know you are not able to merge this, but would you review it since you're familiar with the issue? I think that will help another maintainer.

@ev-br
Copy link
Member

ev-br commented Oct 31, 2022

LGTM as a stop-gap. When this is merged, please list it in gh-17278

Copy link
Contributor

@dpoerio dpoerio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effectively undoes the previous PR (#11712) as intended. I am not currently certain if all of the assertions in the test case are valid (i.e., if the result should be the same as SLSQP algorithm) but that could be revisited once a fix is in place if needed.

@mdhaber
Copy link
Contributor Author

mdhaber commented Oct 31, 2022

The minimal objective function values should be close if they end up in the same local minimum. I understand that if there are multiple local minima they might not, but it is a reasonable smoke test to see if trust-constr is misbehaving. Currently, trust-constr ends up with a lower function value than SLSQP, which prompted me to check the constraints (and the equality constraint is violated).

Copy link
Member

@WarrenWeckesser WarrenWeckesser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. If no one else has jumped in to merge this sooner, and there are no new comments, I'll merge this on Sunday.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Items related to regular maintenance tasks scipy.optimize
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants