Add support for indicator constraints in MPSolver#132
Open
Conversation
Signed-off-by: Peter Mitri <peter.mitri@rte-france.com>
Signed-off-by: Peter Mitri <peter.mitri@rte-france.com>
Signed-off-by: Peter Mitri <peter.mitri@rte-france.com>
Collaborator
Author
Signed-off-by: Peter Mitri <peter.mitri@rte-france.com>
Signed-off-by: Peter Mitri <peter.mitri@rte-france.com>
Signed-off-by: Peter Mitri <peter.mitri@rte-france.com>
flomnes
reviewed
Jul 17, 2024
Collaborator
flomnes
left a comment
flomnes
left a comment
There was a problem hiding this comment.
What happens if indicator_variable is not a binary variable ?
Comment on lines
+446
to
+447
| /// Creates a named indicator constraint with given bounds and given | ||
| /// indicator variable. |
Collaborator
There was a problem hiding this comment.
Suggested change
| /// Creates a named indicator constraint with given bounds and given | |
| /// indicator variable. | |
| /// Creates a named indicator constraint with given bounds and given | |
| /// indicator variable. | |
| /// The constraint is active if and only if *indicator_variable has value indicator_value | |
| /// (Only available for MILP problems) |
Comment on lines
+1514
to
+1515
| MPConstraint* MPSolver::MakeIndicatorConstraint( | ||
| double lb, double ub, const std::string& name, |
Collaborator
There was a problem hiding this comment.
- This method should fail if the solver doesn't support integer variables
- You can call
MPVariable::integerto check it's an integer variable on the indicator variable, and possibly test it's bounds (0 & 1).
| const int constraint_index = NumConstraints(); | ||
| MPConstraint* const constraint = | ||
| new MPConstraint(constraint_index, lb, ub, name, interface_.get()); | ||
| // TODO: check that variable is boolean? |
flomnes
reviewed
Jul 17, 2024
| } | ||
| constraints_.push_back(constraint); | ||
| constraint_is_extracted_.push_back(false); | ||
| interface_->AddIndicatorConstraint(constraint); |
flomnes
reviewed
Jul 17, 2024
| constraint->indicator_variable_ = indicator_variable; | ||
| constraint->indicator_value_ = indicator_value; | ||
| if (!interface_->AddIndicatorConstraint(constraint)) { | ||
| LOG(ERROR) << "Solver doesn't support indicator constraints"; |
Collaborator
There was a problem hiding this comment.
I don't think it's necessary to display a log message here, it's already done in the base version of AddIndicatorConstraint (linear_solver.cc)
virtual bool AddIndicatorConstraint(MPConstraint* const /*ct*/) {
LOG(ERROR) << "Solver doesn't support indicator constraints.";
return false;
}It would be displayed twice. Not terrible, but not great either.
Collaborator
Author
There was a problem hiding this comment.
OK, but then for interfaces that can sometimes handle indicator constraints, sometimes not (for example an interface that can handle MIP or LP problems), we'll have to log the issue before returning "false"
Co-authored-by: Florian Omnès <florian.omnes@rte-france.com>
flomnes
reviewed
Jul 17, 2024
| MPConstraint* MPSolver::MakeIndicatorConstraint( | ||
| double lb, double ub, const std::string& name, | ||
| const MPVariable* indicator_variable, bool indicator_value) { | ||
| if (!indicator_variable->integer() || indicator_variable->lb() != 0 || |
Collaborator
There was a problem hiding this comment.
To be consistent with e.g MPObjective::SetCoefficient and MPConstraint::SetCoefficient, you'd need the following lines
Although the 2nd line is currently included in the 1st.
DLOG_IF(DFATAL, !interface_->solver_->OwnsVariable(indicator_variable)) << indicator_variable;
if (indicator_variable == nullptr) return nullptr;
flomnes
approved these changes
Jul 18, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.