-
Notifications
You must be signed in to change notification settings - Fork 9
Add ipopt method for AbstractNLSModel #132
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
Add ipopt method for AbstractNLSModel #132
Conversation
- Implement ipopt(nls::AbstractNLSModel) as requested in issue JuliaSmoothOptimizers#131 - Add FeasibilityFormNLS function to convert NLS models for optimization - Add comprehensive tests for AbstractNLSModel support - Export FeasibilityFormNLS function for user access Resolves JuliaSmoothOptimizers#131
tmigot
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 @arnavk23 for the PR! I made a first round of comments.
- Remove local FeasibilityFormNLS implementation - Import FeasibilityFormNLS from NLPModelsModifiers package - Update tests to work with the external implementation - Add NLPModelsModifiers to dependencies
1d0fae7 to
b5eb23e
Compare
Co-authored-by: Tangi Migot <[email protected]>
Co-authored-by: Tangi Migot <[email protected]>
Co-authored-by: Dominique <[email protected]>
Co-authored-by: Dominique <[email protected]>
Co-authored-by: Dominique <[email protected]>
Co-authored-by: Dominique <[email protected]>
Co-authored-by: Dominique <[email protected]>
tmigot
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 for the hard work @arnavk23 ! I made some more comments :).
Co-authored-by: Tangi Migot <[email protected]>
Co-authored-by: Tangi Migot <[email protected]>
|
@tmigot Passing all now. I think this can be merged now. |
tmigot
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 @arnavk23 ! I made a final batch of comments and then it will be good to go
fe03af9 to
23c0785
Compare
dpo
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.
Many thanks @arnavk23 !
Co-authored-by: Tangi Migot <[email protected]>
Co-authored-by: Tangi Migot <[email protected]>
Co-authored-by: Tangi Migot <[email protected]>
Co-authored-by: Tangi Migot <[email protected]>
Co-authored-by: Tangi Migot <[email protected]>
tmigot
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 !
Closes #131