Adjust basis repair for super basic variables. Repeat root solve if cuts make it infeasible#831
Conversation
📝 WalkthroughWalkthroughExtended basis repair to track superbasic variables, added superbasic classification and propagation across crossover/primal/dual flows, updated basis repair/update call sites to accept a superbasic_list, and adjusted branch-and-bound root recovery and iteration/objective accounting. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cpp/src/dual_simplex/basis_solves.cpp (1)
627-628:⚠️ Potential issue | 🟡 MinorAssertion may fail when superbasic variables exist.
The assertion
assert(nonbasic_list.size() == n - m)assumes all non-basic variables are innonbasic_list. However, with the introduction of superbasic support, whensuperbasic_listis non-empty, this invariant breaks sincenonbasic_list.size() + superbasic_list.size() == n - m.🐛 Proposed fix
const i_t m = A.m; const i_t n = A.n; assert(basis_list.size() == m); -assert(nonbasic_list.size() == n - m); +assert(nonbasic_list.size() + superbasic_list.size() == n - m);
|
/ok to test a3740e0 |
aliceb-nv
left a comment
There was a problem hiding this comment.
Thank you so much Chris!
|
/ok to test 65af246 |
|
/ok to test d19b30f |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@cpp/src/dual_simplex/branch_and_bound.cpp`:
- Around line 2006-2025: When the recovery solve in
solve_linear_program_with_advanced_basis succeeds, update the cached root
objective and iteration counts: after detecting scratch_status ==
lp_status_t::OPTIMAL (inside the block that sets cut_status =
convert_lp_status_to_dual_status(scratch_status)), recompute root_objective_
from the recovered root_relax_soln_ (so local_lower_bounds_.assign(...,
root_objective_) uses the fresh value) and add the iterations performed by the
recovery solve to exploration_stats_.total_lp_iters; ensure any other derived
quantities depending on the root solution (e.g., gap calculations reported
later) are consistent with the new root_objective_.
|
/ok to test 48b9e88 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@cpp/src/dual_simplex/branch_and_bound.cpp`:
- Around line 1994-1996: The iteration count is coming from the out-parameter
iter returned by dual_phase2_with_advanced_basis, not from
root_relax_soln_.iterations, so replace uses of root_relax_soln_.iterations for
accounting with iter and then store iter into root_relax_soln_.iterations;
specifically update exploration_stats_.total_lp_iters to add iter instead of
root_relax_soln_.iterations and set root_relax_soln_.iterations = iter after
calling dual_phase2_with_advanced_basis so future code sees the populated value.
|
/ok to test 4e1d35c |
|
/merge |
Fixes an issue on neos-4413714-turia where basis repair was called during dual push, and the slack needed was superbasic.
Fixes an issue where cbs-cta was incorrectly classified as infeasible after cuts were added.