-
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 "mixed" standard form representation #3201
Conversation
@@ -372,7 +384,30 @@ def write(self, model): | |||
f"model contains a trivially infeasible constraint, '{con.name}'" | |||
) | |||
|
|||
if slack_form: | |||
if mixed_form: |
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.
Is it more likely that a user will want mixed_form
over slack_form
?
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.
Users will probably do the regular form first, and then maybe the slack form. However, I am working on some (more efficient) solver interfaces (a rewrite of gurobi_direct
) that will use the mixed_form
.
# Note that the solution is a mix of inequality and equality constraints | ||
# self._verify_solution(soln, repn, False) |
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.
Debugging?
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.
Not debugging: a breadcrumb as to why that test is not an exact copy-and-paste of the cases above. I have expanded the 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.
This looks good--I'm excited about it! :) My only big question is that I had a little argument with myself about multiple objectives... I feel like you're probably right, but the idea of more than one objective in "standard form" seems a little terrifying.
active_var_idx[-1] = A.indptr[-1] | ||
# active_var_idx[-1] = len(columns) | ||
A = scipy.sparse.csc_array( |
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.
Did you intend to leave this comment?
) | ||
self.assertTrue(np.all(repn.A == ref)) | ||
self.assertTrue(np.all(repn.b == np.array([3, 5, 6, -3, 8]))) | ||
self.assertTrue(np.all(repn.c == np.array([[-1, 0, -5, 0], [1, 0, 0, 15]]))) |
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.
EEEEEK, I didn't notice this before--should we really let a "standard form" have more than one objective? This caused me a moment of terror on the the thought that I would definitely assume c
is a vector of the same dimension as the number of columns in A
... The matrix c
makes sense to me from a writer perspective. From a user/transformation perspective, we're always going to need to be checking that this isn't the case... But I guess we have to do that anyway, so why not here?
Fixes # .
Summary/Motivation:
This adds a new option to the "Standard Form" compiler that returns a mix of inequality and equality constraints. This representation is particularly useful for solver interfaces. This PR also resolves an error when collapsing unused variables out of the returned matrices. The new implementation is also slightly more performant.
Changes proposed in this PR:
A
matrix andc
vectorLegal Acknowledgement
By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution: