Convert timed mipgap to use termination callback mechanics - #810
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates mpi-sppy’s time-dependent MIP gap feature to rely on the unified “termination callback” infrastructure (instead of solver-specific callback mechanics), and expands/normalizes termination-callback behavior across supported persistent solvers.
Changes:
- Refactors
TimedMIPGapCBto register a generic termination callback (CPLEX/Gurobi/Xpress persistent) and adds robust parsing/validation of thegap:timecurve. - Enhances termination-callback support with subclass-aware dispatch and improved solver-result status normalization (notably for CPLEX and Gurobi).
- Adds targeted unit/integration tests and documents the new CLI flags and extension behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| mpisppy/utils/callbacks/termination/tests/test_termination.py | Adds tests for subclass dispatch and solver status reporting for termination callbacks. |
| mpisppy/utils/callbacks/termination/termination_callbacks.py | Implements setter lookup via isinstance (subclass support) and improves unsupported-solver error path. |
| mpisppy/utils/callbacks/termination/solver_callbacks.py | Updates solver-specific callback wiring; normalizes postsolve statuses for callback-triggered stops (CPLEX/Gurobi) and updates Xpress termination mechanism. |
| mpisppy/tests/test_timed_mipgap.py | Adds parsing/unit tests plus solver-backed integration checks for timed MIP gap termination. |
| mpisppy/extensions/timed_mipgap.py | Refactors timed MIP gap to use termination callbacks and validates the timecurve input. |
| doc/src/generic_cylinders.rst | Documents --timed-mipgap and --timed-mipgap-options flags. |
| doc/src/extensions.rst | Adds detailed documentation for the timed MIP gap extension, its semantics, and solver requirements. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #810 +/- ##
==========================================
+ Coverage 76.91% 77.76% +0.84%
==========================================
Files 177 177
Lines 23646 23764 +118
==========================================
+ Hits 18188 18480 +292
+ Misses 5458 5284 -174 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
DLWoodruff
left a comment
DLWoodruff
left a comment
There was a problem hiding this comment.
config.checker needs two updates:
0. --timed-mipgap-options given without --timed-mipgap is an error (does nothing; alternatively, you could make it do something)
- fwph_obj_gab_hub is a hub
Not sure about (1) (at least for this PR), but for (0) we went with just |
tvalenciaz
left a comment
tvalenciaz
left a comment
There was a problem hiding this comment.
I only tested gurobi and xpress because I don't have CPLEX in my system. I had an issue with xpress, probably due to having to use an older version (9.7.1), but I think it would be easy to keep it compatible with both versions. Otherwise looks good to me!
| # Per the Xpress documentation, this callback is invoked every time the | ||
| # Optimizer checks if the time limit has been reached. This is broader than | ||
| # what is presently needed for our MIP-based use cases. | ||
| xpress_problem.addCheckTimeCallback( |
There was a problem hiding this comment.
Using xpress 9.7.1 (the most recent my license allows), I had to change this to xpress_problem.addcbchecktime( for it to work.
I know from the email thread that this was running for you so it's probably due to the older version. Do we want to check which of addcbchecktime or addCheckTimeCallback exists and call that one?
There was a problem hiding this comment.
Yes, we can check if addCheckTimeCallback exists, and if it does, call it, and if it doesn't call addcbchecktime.
No description provided.