-
Notifications
You must be signed in to change notification settings - Fork 518
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 support for cyipopt #2830
add support for cyipopt #2830
Conversation
@ZedongPeng, it looks like you need to run black on this. (It needs to pass that before the other tests will run.) The command is |
Hi @emma58 . Where to add the |
@ZedongPeng, you want to run that command from the commandline on this branch, and then push the result. (You can install black with pip or conda, I think.) |
@ZedongPeng - Correct. You can install |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #2830 +/- ##
==========================================
+ Coverage 87.45% 87.47% +0.01%
==========================================
Files 771 771
Lines 89579 89646 +67
==========================================
+ Hits 78342 78416 +74
+ Misses 11237 11230 -7
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
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.
Good PR. I left some comments with small changes in the comments and class names. Maybe we can add more tests exercising the fact that we have an extra solver now (and, for example trying some non-supported solver name just in case).
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.
I agree with @bernalde that adding some tests would be good--in particular you don't have any tests of the special handling for cyipopt duals. But otherwise this looks good.
732ab31
to
90117ea
Compare
Summary/Motivation:
Add the support for cyipopt as a NLP solver in MindtPy.
Related issue: #2831
Legal Acknowledgement
By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution: