-
Notifications
You must be signed in to change notification settings - Fork 562
Add option to ignore inequality constraints in IncidenceGraphInterface #2350
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
Conversation
michaelbynum
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 @Robbybp!
Codecov Report
@@ Coverage Diff @@
## main #2350 +/- ##
==========================================
- Coverage 85.83% 83.85% -1.98%
==========================================
Files 617 617
Lines 75949 75952 +3
==========================================
- Hits 65191 63693 -1498
- Misses 10758 12259 +1501
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Removed WIP label. Ready for review/merge, assuming tests pass. |
| if include_inequality: | ||
| self.incidence_matrix = nlp.evaluate_jacobian() | ||
| else: | ||
| self.incidence_matrix = nlp.evaluate_jacobian_eq() |
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.
This changes behavior. Previously the nlp interface would always ignore inequalities, and the model interface would never ignore inequalities. This patch makes the two interfaces consistent (by default, include inequalities) and adds the option to ignore inequalities, which applies to both interface.
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'm not too worried about this because (a) this is contrib and (b) I'm not aware of anybody using the NLP interface.
Summary/Motivation:
Often when analyzing an incidence matrix/graph, we are interested in the structure of equations and variables, and would like to exclude inequality constraints from the analysis. This can be done right now by passing equality constraints directly to the methods one wants to use, e.g.
maximum_matching, but this is cumbersome. This PR adds aninclude_inequalityflag to the constructor that, when False, only considers constraints whose expressions are instances ofEqualityExpression.Opened this PR as it has come to my attention that this might be useful. Marked WIP for now I haven't included a test for this functionality yet.
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: