Add more judicious enforcement of PyROS Solver time limit #2660
Conversation
|
@jsiirola this PR is ready for review |
Codecov ReportBase: 87.06% // Head: 87.06% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #2660 +/- ##
=======================================
Coverage 87.06% 87.06%
=======================================
Files 757 757
Lines 84513 84585 +72
=======================================
+ Hits 73583 73647 +64
- Misses 10930 10938 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
Overall, this looks reasonable. In addition to the question and change below, can you also add a test that exercises the time_limit option (and improves the code coverage for processing timeouts)? That might be tricky (you may need to create a custom "mock solver" that does nothing but wait until the timeout and returns a prearranged state).
pyomo/contrib/pyros/util.py
Outdated
| if isinstance(solver, type(SolverFactory("baron"))): | ||
| options_key = "MaxTime" | ||
| elif isinstance(solver, type(SolverFactory("ipopt"))): | ||
| options_key = "max_cpu_time" |
There was a problem hiding this comment.
Nothing wrong with this implementation, but it is slightly more efficient to use
if isinstance(solver, SolverFactory.get_class('baron')):(This prevents creating / throwing away solver instances)
| # ensure positive value assigned to avoid application error | ||
| solver.options[options_key] = max(30, 30 + time_remaining) |
There was a problem hiding this comment.
30 is somewhat random. Can you add a comment with some justification for selecting this as the minimal timeout(and why it is not - say - 1, 5, or 120)?
Changes proposed in this PR:
time_limitpassed to PyROS and the time elapsed. This time limit adjustment method will be supported for'gams','ipopt', and'baron'.Todo (after #2659 merged)
pyros.pysuch that subsolvers are copied/cloned on argument parsing, so that state of solver(s) passed by user remain unaffected.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: