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

ENH: expose 'eps' option for finite difference step size #228

Merged
merged 2 commits into from
Nov 5, 2023

Conversation

mdhaber
Copy link
Contributor

@mdhaber mdhaber commented Sep 25, 2023

closes gh-115, which requested that eps, the finite difference step size, be exposed as an option.

Comment on lines +1128 to +1134
assert res.fun <= ref.fun # Ipopt legitimately does better than slsqp here

# If we give SLSQP a good guess, it agrees with Ipopt
ref2 = minimize(fun=obj, x0=res.x, method='slsqp',
bounds=bnds, constraints=nlcs)
assert ref2.success
assert_allclose(ref2.fun, res.fun)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: this change is unrelated to the rest of the PR, but since I needed to re-run CI, I decided to do it here rather than opening another PR.

While developing gh-207, I noticed two things about minimize_ipopt that were concerning. One is addressed by gh-229; this addresses the other. I wasn't sure before, but I see now that Ipopt really does find a better solution to this problem than SLSQP. This extension confirms that the solution found by Ipopt is legitimate because SLSQP agrees when the solution is provided as a guess.

@moorepants moorepants merged commit 97c9ec7 into mechmotum:master Nov 5, 2023
34 checks passed
@moorepants
Copy link
Collaborator

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add eps argument or passable option to minimize_ipopt()
2 participants