-
Notifications
You must be signed in to change notification settings - Fork 47
Unified Interface: Absolute Tolerance #1660
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
base: develop
Are you sure you want to change the base?
Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1660 +/- ##
===========================================
- Coverage 84.37% 84.35% -0.02%
===========================================
Files 164 164
Lines 14320 14354 +34
===========================================
+ Hits 12082 12109 +27
- Misses 2238 2245 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
closes #1646 |
dweindl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @PaulJonasJost. Good feature to add, but I think this requires a few changes:
It's currently unclear
- whether it's an absolute or relative tolerance. (included in the docstring, but not in the method name. we might want to support both in the future, so the method name should be unambiguous.)
- what the tolerance applies to. many gradient-based optimizers will support tolerances on the objective as well as its gradient, so it needs to become clear which one is specified here.
| tol | ||
| Absolute tolerance for termination. | ||
| """ | ||
| self._set_option_tol(tol, "tol") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I remember, ipopt has quite complex termination criteria. While various tolerances are supported, I think just hitting this single value is insufficient for termination, so it might be a bit confusing. Not completely sure whether it should be added here or not.
Add tolerance to the unified interface. Some optimizers not supported, especially Dlib with epsilon, though epsilon is not strictly a tolerance!