-
Notifications
You must be signed in to change notification settings - Fork 18
Add HiGHS.ComputeInfeasibilityCertificate attribute #274
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
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #274 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 1
Lines 1471 1479 +8
=========================================
+ Hits 1471 1479 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
cc @sstroemer thoughts? |
sstroemer
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.
Noticed two minor things, but looks good to me - thanks for adding that specifically!!
src/MOI_wrapper.jl
Outdated
| x.model_status = Highs_getModelStatus(model) | ||
| statusP = Ref{HighsInt}() | ||
| if x.model_status == kHighsModelStatusInfeasible | ||
| certificates = MOI.get(model, ComputeInfeasibilityCertificates()) |
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.
One thing I noticed: Setting ComputeInfeasibilityCertificates() to false will always result in both x.has_dual_ray and x.has_primal_ray being false.
Therefore that will never trigger:
if x.has_dual_ray || x.has_primal_ray
return # If a ray is present, we don't query the solution
endI have no idea if querying the solution (status) afterwards costs time or whether skipping it also when !certificates && (x.model_status == kHighsModelStatusInfeasible || x.model_status == kHighsModelStatusUnbounded) would even be correct, but just a thought.
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.
We query the solution only if x.primal_solution_status != kHighsSolutionStatusNone, which won't be the case for your infeasible problems. I'd like to leave the behavior as-is.
Closes #273