Catch catastrophic solver failure when building MIS#3403
Conversation
| results = None | ||
|
|
||
| if pyo.check_optimal_termination(results): | ||
| if (results is not None) and pyo.check_optimal_termination(results): |
There was a problem hiding this comment.
Question: would it better to not swallow all exceptions from the solve() above and instead use load_solutions=False in the call? Then results should never be None?
There was a problem hiding this comment.
Probably. Would that still not raise an exception if the solver returns a non-zero exit code? I think that's why I've implemented these as blanket try / except blocks.
There was a problem hiding this comment.
OK. I would just be worried about the bare except: -- that could be suppressing any number of errors and there would be no way to know what was going on.
jsiirola
left a comment
jsiirola
left a comment
There was a problem hiding this comment.
I am OK merging this as it resolves a serious error blocking MIS users. However, this highlights fragility in the MIS implementation due to the use of a bare (catch-all) "except:" clause.
It would be good to revisit this and see if the bare "except:" can be removed.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3403 +/- ##
==========================================
+ Coverage 88.63% 88.66% +0.03%
==========================================
Files 879 879
Lines 100363 100363
==========================================
+ Hits 88955 88986 +31
+ Misses 11408 11377 -31
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
5 participants
Fixes IDAES/idaes-pse#1520
Summary/Motivation:
The minimal intractable system calculation had a try/except around a solve we expect to fail, but when we check for failure we failed to first check the results object against
None.Changes proposed in this PR:
results is not Noneconditional to short-circuit, thus not callingpyo.check_optimal_termination(results)ifresults is None.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: