Repository navigation
Conversation
Pass unset variables to HiGHS as kHighsUndefined instead of 0, so HiGHS can complete a partial start rather than rejecting it, and add a test.
jsiirola
left a comment
There was a problem hiding this comment.
This looks really good. I have one nit-picky question around how the test is guarded, but otherwise this looks good.
| @unittest.skipUnless( | ||
| hasattr(highspy, "kHighsUndefined"), | ||
| "Partial MIP starts require highspy>=1.11 (kHighsUndefined)", | ||
| ) |
There was a problem hiding this comment.
Style question: should this guard be based on the existence of the attribute or based on the HiGHS version? My only concerns with looking for the attribute are:
- "assume we mistyped the attribute - it would never be found and the test would never be run - and we would never know because it wouldn't show up on the coverage (because we only look for line coverage and not branch coverage).
- we would not see if a new HiGHS release removed / renamed that attribute, because we would just silently stop testing the code.
There was a problem hiding this comment.
Thank you, good point. I chose this approach because I wanted to make sure the test is run whenever the attribute is available, which I deemed more robust than checking the version, but I think your argument is stronger. I will adapt this.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4046 +/- ##
==========================================
+ Coverage 89.96% 90.14% +0.17%
==========================================
Files 917 917
Lines 109282 109282
==========================================
+ Hits 98311 98507 +196
+ Misses 10971 10775 -196
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Fixes
Part 1 of #4044
Summary/Motivation:
appsi_highsbuilds the MIP start vector withnp.zeros, so every variable without a value is passed to HiGHS as 0. That turns a partial start into a complete assignment, which can easily be infeasible and get rejected. HiGHS has supported partial starts since 1.8: entries set tokHighsUndefinedare left free, and HiGHS completes the start by solving a sub-MIP over the unset discrete variables.Changes proposed in this PR:
Highs._warm_startwithhighspy.kHighsUndefinedinstead of 0. On highspy < 1.11, which does not export the constant, it falls back to 0.0 (the current behaviour).test_partial_warm_start: a small MIP where the unset binary has to be 1 for the start to be feasible. With the change, HiGHS reports the completed start as feasible. Without it, the zero-filled start is rejected. The test is skipped on highspy < 1.11 (via a check for the existence of thekHighsUndefinedattribute).AI-Use Disclosure
or
AI tools contributed to the development of this PR
Review process (select ONE):
Notes for reviewers (optional):
The new test reuses the model from
test_warm_startand adds one constraint,x1 <= 3 + 7 * x3, so that withx1 = 4the unset binary must be 1. Without the fix,x3is passed as 0, the start is infeasible and gets rejected. The test only checks forMIP start solution is feasibleand leaves out the objective value, because that value depends on how HiGHS completes the partial start.C5is deliberately loose forx3 = 1: with a tighter version such asx1 <= 3 + x3, HiGHS presolve solves the model before the start is evaluated, and the log line never appears. I verified the test with highspy 1.15.1 only.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: