Fix incorrect infeasible list

[chris-maes]

This fixes two issues that could cause dual simplex infeasible list to incorrect:

1. When recomputing the primal infeasibilities we need to clear squared_infeasibilites
2. When performing basis repair we need to correctly set the variable that we remove from the basis to be on a bound.

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Enhanced variable status assignment during basis repair to consider variable bounds, selecting appropriate status based on constraint availability.
  • Improved primal infeasibility tracking to maintain both linear and squared infeasibility sums for more accurate solver diagnostics.

• Refactor

  • Updated basis repair and update operations throughout the solver to accept and propagate variable bound vectors for bound-aware processing.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[chris-maes]

/ok to test f341e34

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR extends the basis repair and basis update mechanisms in the dual simplex solver to accept variable bounds vectors, enabling bounds-aware repair decisions. Function signatures for basis_repair and refactor_basis are updated across headers and implementations, with corresponding call sites updated to pass lower and upper bound information from the problem instance.

Changes

Cohort / File(s)	Summary
Header Declarations cpp/src/dual_simplex/basis_solves.hpp, cpp/src/dual_simplex/basis_updates.hpp	Updated function declarations to include const std::vector<f_t>& lower and const std::vector<f_t>& upper parameters. basis_repair adds bounds after settings; refactor_basis in basis_update_mpf_t similarly incorporates bounds.
Bounds-Aware Repair Logic cpp/src/dual_simplex/basis_solves.cpp	Implemented bounds-aware status assignment in basis_repair: selects NONBASIC_FREE for infinite bounds, NONBASIC_LOWER for finite lower bound, NONBASIC_UPPER for finite upper bound. Updated function signature and template instantiation to accept and use lower/upper vectors.
Refactor Basis Signatures cpp/src/dual_simplex/basis_updates.cpp	Added lower and upper parameters to refactor_basis methods in both basis_update_t and basis_update_mpf_t classes. Updated error path to propagate bounds into basis_repair calls during factorization failures.
Bounds Propagation at Call Sites cpp/src/dual_simplex/crossover.cpp, cpp/src/dual_simplex/primal.cpp	Updated basis_repair calls to pass lp.lower and lp.upper alongside existing arguments across multiple code paths, enabling repair operations to leverage bound information.
Complex Infeasibility Refactoring cpp/src/dual_simplex/phase2.cpp	Multiple changes: (1) refactor_basis calls now include lp.lower and lp.upper parameters; (2) compute_initial_primal_infeasibilities signature updated to output f_t& primal_inf parameter and return squared infeasibility sum; (3) infeasibility tracking refactored to maintain both linear and squared sums; (4) call sites updated to use new return value and output parameter.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

• basis_solves.cpp: Verify bounds-aware logic correctly assigns status based on bound finiteness conditions and handles all cases
• phase2.cpp: Review the compute_initial_primal_infeasibilities signature change and return type modification; ensure all call sites correctly handle the new squared infeasibility return value and the new output parameter
• Call site consistency: Confirm all calls to basis_repair and refactor_basis across the module correctly pass bounds vectors in the proper order
• Integration: Verify that bounds flow correctly from problem instance through repair operations and that no edge cases are missed in the bounds-aware assignment logic

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Fix incorrect infeasible list' directly relates to the core fixes described: clearing squared_infeasibilities and setting correct bounds during basis repair to ensure the infeasible list is accurate.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

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 (2)

cpp/src/dual_simplex/basis_solves.cpp (1)

614-671: Handle fixed variables in basis_repair (and assert bounds sizes). The new bound-aware status assignment (Lines 663-671) is the right direction, but it misclassifies fixed variables (lower == upper) as NONBASIC_LOWER. That can cascade into incorrect bound placement / infeasibility tracking. Also consider guarding against bounds vector size mismatches.

 template <typename i_t, typename f_t>
 i_t basis_repair(const csc_matrix_t<i_t, f_t>& A,
                  const simplex_solver_settings_t<i_t, f_t>& settings,
                  const std::vector<f_t>& lower,
                  const std::vector<f_t>& upper,
                  const std::vector<i_t>& deficient,
                  const std::vector<i_t>& slacks_needed,
                  std::vector<i_t>& basis_list,
                  std::vector<i_t>& nonbasic_list,
                  std::vector<variable_status_t>& vstatus)
 {
   const i_t m = A.m;
   const i_t n = A.n;
   assert(basis_list.size() == m);
   assert(nonbasic_list.size() == n - m);
+  assert(lower.size() == static_cast<size_t>(n));
+  assert(upper.size() == static_cast<size_t>(n));
 ...
     vstatus[replace_j]                     = variable_status_t::BASIC;
-    // This is the main issue. What value should bad_j take on.
-    if (lower[bad_j] == -inf && upper[bad_j] == inf) {
+    // Set removed basic variable to an appropriate nonbasic bound status.
+    if (std::abs(upper[bad_j] - lower[bad_j]) < settings.fixed_tol) {
+      vstatus[bad_j] = variable_status_t::NONBASIC_FIXED;
+    } else if (lower[bad_j] == -inf && upper[bad_j] == inf) {
       vstatus[bad_j] = variable_status_t::NONBASIC_FREE;
     } else if (lower[bad_j] > -inf) {
       vstatus[bad_j] = variable_status_t::NONBASIC_LOWER;
     } else if (upper[bad_j] < inf) {
       vstatus[bad_j] = variable_status_t::NONBASIC_UPPER;
     } else {
       assert(1 == 0);
     }
   }

cpp/src/dual_simplex/phase2.cpp (1)

653-691: Bug: update_single_primal_infeasibility updates a squared accumulator but is called with the linear accumulator.
update_single_primal_infeasibility computes old_val/new_val as squared infeasibilities and applies (new_val - old_val) to its primal_inf reference, but the call at Line 2832+ passes primal_infeasibility (linear sum). This will corrupt the linear metric (and can go negative/oscillate despite max(0, ...)).

Minimal fix (keep update_single_primal_infeasibility as “squared” updater, consistent with the other two calls):

@@
     phase2::update_single_primal_infeasibility(lp.lower,
                                                lp.upper,
                                                x,
                                                settings.primal_tol,
                                                squared_infeasibilities,
                                                infeasibility_indices,
                                                entering_index,
-                                               primal_infeasibility);
+                                               primal_infeasibility_squared);

Follow-up (recommended): rename the parameter to primal_inf_squared in update_single_primal_infeasibility / update_primal_infeasibilities to prevent future mixups.

Also applies to: 2809-2840

🧹 Nitpick comments (1)

cpp/src/dual_simplex/basis_updates.cpp (1)

2046-2072: Add defensive size checks (and consider checking the basis_repair return) before using bounds.
Right now lower/upper are assumed consistent with A.n/vstatus, and basis_repair(...)’s return is ignored—fine if guaranteed, but brittle if upstream ever violates invariants. As per coding guidelines/learnings, validating bounds/state at phase transitions is important.

@@
 int basis_update_mpf_t<i_t, f_t>::refactor_basis(
   const csc_matrix_t<i_t, f_t>& A,
   const simplex_solver_settings_t<i_t, f_t>& settings,
   const std::vector<f_t>& lower,
   const std::vector<f_t>& upper,
   std::vector<i_t>& basic_list,
   std::vector<i_t>& nonbasic_list,
   std::vector<variable_status_t>& vstatus)
 {
+  assert(lower.size() == static_cast<size_t>(A.n));
+  assert(upper.size() == static_cast<size_t>(A.n));
+  assert(vstatus.size() == static_cast<size_t>(A.n));
+
@@
   if (factorize_basis(...) == -1) {
     settings.log.debug("Initial factorization failed\n");
-    basis_repair(A, settings, lower, upper, deficient, slacks_needed, basic_list, nonbasic_list, vstatus);
+    const auto repair_rc =
+      basis_repair(A, settings, lower, upper, deficient, slacks_needed, basic_list, nonbasic_list, vstatus);
+    (void)repair_rc; // or handle if non-zero is meaningful

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d97ff6b and f341e34.

📒 Files selected for processing (7)

• cpp/src/dual_simplex/basis_solves.cpp (3 hunks)
• cpp/src/dual_simplex/basis_solves.hpp (1 hunks)
• cpp/src/dual_simplex/basis_updates.cpp (2 hunks)
• cpp/src/dual_simplex/basis_updates.hpp (1 hunks)
• cpp/src/dual_simplex/crossover.cpp (3 hunks)
• cpp/src/dual_simplex/phase2.cpp (12 hunks)
• cpp/src/dual_simplex/primal.cpp (1 hunks)

🧰 Additional context used

📓 Path-based instructions (4)

**/*.{cu,cuh,cpp,hpp,h}

📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)

**/*.{cu,cuh,cpp,hpp,h}: Track GPU device memory allocations and deallocations to prevent memory leaks; ensure cudaMalloc/cudaFree balance and cleanup of streams/events
Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
Check numerical stability: prevent overflow/underflow, precision loss, division by zero/near-zero, and use epsilon comparisons for floating-point equality checks
Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
For concurrent CUDA operations (barriers, async operations), explicitly create and manage dedicated streams instead of reusing the default stream; document stream lifecycle
Eliminate unnecessary host-device synchronization (cudaDeviceSynchronize) in hot paths that blocks GPU pipeline; use streams and events for async execution
Assess algorithmic complexity for large-scale problems (millions of variables/constraints); ensure O(n log n) or better complexity, not O(n²) or worse
Verify correct problem size checks before expensive GPU/CPU operations; prevent resource exhaustion on oversized problems
Identify assertions with overly strict numerical tolerances that fail on legitimate degenerate/edge cases (near-zero pivots, singular matrices, empty problems)
Ensure race conditions are absent in multi-GPU code and multi-threaded server implementations; verify proper synchronization of shared state
Refactor code duplication in solver components (3+ occurrences) into shared utilities; for GPU kernels, use templated device functions to avoid duplication
Check that hard-coded GPU de...

Files:

• cpp/src/dual_simplex/basis_updates.hpp
• cpp/src/dual_simplex/primal.cpp
• cpp/src/dual_simplex/crossover.cpp
• cpp/src/dual_simplex/basis_solves.cpp
• cpp/src/dual_simplex/basis_updates.cpp
• cpp/src/dual_simplex/basis_solves.hpp
• cpp/src/dual_simplex/phase2.cpp

**/*.{h,hpp,py}

📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)

Verify C API does not break ABI stability (no struct layout changes, field reordering); maintain backward compatibility in Python and server APIs with deprecation warnings

Files:

• cpp/src/dual_simplex/basis_updates.hpp
• cpp/src/dual_simplex/basis_solves.hpp

**/*.{cpp,hpp,h}

📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)

**/*.{cpp,hpp,h}: Check for unclosed file handles when reading MPS/QPS problem files; ensure RAII patterns or proper cleanup in exception paths
Validate input sanitization to prevent buffer overflows and resource exhaustion attacks; avoid unsafe deserialization of problem files
Prevent thread-unsafe use of global and static variables; use proper mutex/synchronization in server code accessing shared solver state

Files:

• cpp/src/dual_simplex/basis_updates.hpp
• cpp/src/dual_simplex/primal.cpp
• cpp/src/dual_simplex/crossover.cpp
• cpp/src/dual_simplex/basis_solves.cpp
• cpp/src/dual_simplex/basis_updates.cpp
• cpp/src/dual_simplex/basis_solves.hpp
• cpp/src/dual_simplex/phase2.cpp

**/*.{cu,cpp,hpp,h}

📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)

Avoid inappropriate use of exceptions in performance-critical GPU operation paths; prefer error codes or CUDA error checking for latency-sensitive code

Files:

• cpp/src/dual_simplex/basis_updates.hpp
• cpp/src/dual_simplex/primal.cpp
• cpp/src/dual_simplex/crossover.cpp
• cpp/src/dual_simplex/basis_solves.cpp
• cpp/src/dual_simplex/basis_updates.cpp
• cpp/src/dual_simplex/basis_solves.hpp
• cpp/src/dual_simplex/phase2.cpp

🧠 Learnings (9)

📓 Common learnings

Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Reduce tight coupling between solver components (presolve, simplex, basis, barrier); increase modularity and reusability of optimization algorithms

Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)

📚 Learning: 2025-11-25T10:20:49.822Z

Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Reduce tight coupling between solver components (presolve, simplex, basis, barrier); increase modularity and reusability of optimization algorithms

Applied to files:

• cpp/src/dual_simplex/basis_updates.hpp
• cpp/src/dual_simplex/primal.cpp
• cpp/src/dual_simplex/crossover.cpp
• cpp/src/dual_simplex/basis_solves.cpp
• cpp/src/dual_simplex/basis_updates.cpp
• cpp/src/dual_simplex/basis_solves.hpp
• cpp/src/dual_simplex/phase2.cpp

📚 Learning: 2025-11-25T10:20:49.822Z

Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)

Applied to files:

• cpp/src/dual_simplex/primal.cpp
• cpp/src/dual_simplex/crossover.cpp
• cpp/src/dual_simplex/basis_solves.cpp
• cpp/src/dual_simplex/basis_updates.cpp
• cpp/src/dual_simplex/basis_solves.hpp
• cpp/src/dual_simplex/phase2.cpp

📚 Learning: 2025-11-25T10:20:49.822Z

Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations

Applied to files:

• cpp/src/dual_simplex/primal.cpp
• cpp/src/dual_simplex/basis_solves.cpp
• cpp/src/dual_simplex/phase2.cpp

📚 Learning: 2025-11-25T10:20:49.822Z

Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Add tests for algorithm phase transitions: verify correct initialization of bounds and state when transitioning from presolve to simplex to diving to crossover

Applied to files:

• cpp/src/dual_simplex/crossover.cpp
• cpp/src/dual_simplex/basis_solves.cpp
• cpp/src/dual_simplex/basis_updates.cpp
• cpp/src/dual_simplex/phase2.cpp

📚 Learning: 2025-11-25T10:20:49.822Z

Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results

Applied to files:

• cpp/src/dual_simplex/crossover.cpp
• cpp/src/dual_simplex/basis_solves.cpp
• cpp/src/dual_simplex/basis_updates.cpp
• cpp/src/dual_simplex/basis_solves.hpp
• cpp/src/dual_simplex/phase2.cpp

📚 Learning: 2025-12-04T20:09:09.264Z

Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 602
File: cpp/src/linear_programming/solve.cu:732-742
Timestamp: 2025-12-04T20:09:09.264Z
Learning: In cpp/src/linear_programming/solve.cu, the barrier solver does not currently return INFEASIBLE or UNBOUNDED status. It only returns OPTIMAL, TIME_LIMIT, NUMERICAL_ISSUES, or CONCURRENT_LIMIT.

Applied to files:

• cpp/src/dual_simplex/basis_solves.cpp
• cpp/src/dual_simplex/phase2.cpp

📚 Learning: 2025-12-04T04:11:12.640Z

Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 500
File: cpp/src/dual_simplex/scaling.cpp:68-76
Timestamp: 2025-12-04T04:11:12.640Z
Learning: In the cuOPT dual simplex solver, CSR/CSC matrices (including the quadratic objective matrix Q) are required to have valid dimensions and indices by construction. Runtime bounds checking in performance-critical paths like matrix scaling is avoided to prevent slowdowns. Validation is performed via debug-only check_matrix() calls wrapped in #ifdef CHECK_MATRIX.

Applied to files:

• cpp/src/dual_simplex/basis_solves.cpp

📚 Learning: 2025-11-25T10:20:49.822Z

Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Identify assertions with overly strict numerical tolerances that fail on legitimate degenerate/edge cases (near-zero pivots, singular matrices, empty problems)

Applied to files:

• cpp/src/dual_simplex/basis_solves.cpp

🧬 Code graph analysis (5)

cpp/src/dual_simplex/basis_updates.hpp (1)

cpp/src/dual_simplex/presolve.cpp (3)

• lower (240-240)
• upper (85-85)
• upper (241-241)

cpp/src/dual_simplex/crossover.cpp (2)

cpp/src/dual_simplex/basis_solves.cpp (3)

• basis_repair (614-675)
• basis_repair (614-622)
• basis_repair (860-868)

cpp/src/dual_simplex/basis_solves.hpp (1)

• basis_repair (43-51)

cpp/src/dual_simplex/basis_solves.cpp (1)

cpp/src/dual_simplex/presolve.cpp (3)

• lower (240-240)
• upper (85-85)
• upper (241-241)

cpp/src/dual_simplex/basis_updates.cpp (3)

cpp/src/dual_simplex/basis_solves.cpp (3)

• basis_repair (614-675)
• basis_repair (614-622)
• basis_repair (860-868)

cpp/src/dual_simplex/basis_solves.hpp (1)

• basis_repair (43-51)

cpp/src/dual_simplex/basis_updates.hpp (1)

• A (374-380)

cpp/src/dual_simplex/phase2.cpp (1)

cpp/src/dual_simplex/primal.cpp (2)

• primal_infeasibility (202-240)
• primal_infeasibility (202-205)

🔇 Additional comments (7)

cpp/src/dual_simplex/primal.cpp (1)

301-301: Updated basis_repair call looks correct (bounds propagated). Line 301 now passes lp.lower/lp.upper, aligning with the new bound-aware repair contract.

cpp/src/dual_simplex/crossover.cpp (1)

788-790: All basis_repair call sites updated consistently. These hunks correctly pass lp.lower/lp.upper, matching the updated basis_repair signature.

Also applies to: 1135-1136, 1326-1327

cpp/src/dual_simplex/basis_solves.cpp (1)

860-868: Instantiation updated consistently. Template instantiation now matches the new basis_repair(..., lower, upper, ...) signature.

cpp/src/dual_simplex/basis_solves.hpp (1)

43-51: Header signature matches the new bound-aware repair API. Adds lower/upper in the right position to align with implementations and updated call sites.

cpp/src/dual_simplex/basis_updates.hpp (1)

374-380: Signature update is correctly implemented across all call sites. All three refactor_basis calls in cpp/src/dual_simplex/phase2.cpp (lines 2248, 2891, 2898) correctly pass lp.lower and lp.upper after bounds validation. The assertion checks at lines 2222–2223 ensure lower.size() == upper.size() == n before invocation, satisfying the contract requirements.

cpp/src/dual_simplex/phase2.cpp (2)

621-651: Good fix: squared_infeasibilities is now cleared on recomputation.
This addresses the “stale infeasible list” failure mode and aligns with the “reset algorithm state between phases/recomputations” guidance.

---

2248-2250: Bounds-aware refactor_basis(...) wiring looks consistent.
Passing lp.lower/lp.upper through refactor/repair is the right direction for correct post-repair vstatus classification (removed variable becomes NONBASIC_{LOWER,UPPER,FREE}).

Also applies to: 2891-2902

[nguidotti]

LGTM. I can confirm that this fixes the bound violations in neos-2657525-crna. It has similar results for netlib for simplex, barrier + crossover and pdlp + crossover as the main branch.

[github-actions]

🔔 Hi @anandhkb @chris-maes, this pull request has had no activity for 7 days. Please update or let us know if it can be closed. Thank you!

If this is an "epic" issue, then please add the "epic" label to this issue.
If it is a PR and not ready for review, then please convert this to draft.
If you just want to switch off this notification, then use the "skip inactivity reminder" label.