Fix a bug in dual variables and reduced costs when we add implied bounds on free variables by chris-maes · Pull Request #1237 · NVIDIA/cuopt

chris-maes · 2026-05-18T21:12:07Z

When -inf <= x_j <= inf and we add implied bounds on x_j, we will compute a dual solution (u, w) that satisfies

A^T u + w = c + Qx

but w_j could be nonzero. When x_j is free, we need to enforce that the corresponding reduced cost is zero. We do this by transforming (u, w) into (y, z) such that A^T y + z = c + Qx with z_j = 0 for all j such that -inf < x_j < inf.

We also add a unit test on a small QP to confirm the fix.

Fixes #1119

…nds on free variables When -inf <= x_j <= inf and we add implied bounds on x_j, we will compute a dual solution (u, w) that satisfies A^T u + w = c + Qx but w_j could be nonzero. When x_j is free, we need to enforce that the corresponding reduced cost is zero. We do this by transforming (u, w) into (y, z) such that A^T y + z = c + Qx with z_j = 0 for all j such that -inf < x_j < inf. We also add a unit test on a small QP to confirm the fix.

copy-pr-bot · 2026-05-18T21:12:11Z

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 · 2026-05-18T21:13:46Z

/ok to test 23789ff

chris-maes · 2026-05-18T21:15:13Z

/ok to test 06eb7d2

coderabbitai · 2026-05-18T21:17:12Z

📝 Walkthrough

Walkthrough

This PR tracks which constraints imply finite bounds on formerly-free variables during presolve and applies post-solve dual corrections during uncrush using the original constraint matrix to update duals and reduced costs.

Changes

Barrier dual correction for bounded free variables

Layer / File(s)	Summary
Data structures for bounded free variable tracking `cpp/src/dual_simplex/presolve.hpp`	Introduces `bounded_free_var_t<i_t,f_t>` and adds `presolve_info_t::bounded_free_variables`; updates `uncrush_solution` declaration to accept `original_problem`.
Presolve tracking of bound-originating constraints `cpp/src/dual_simplex/presolve.cpp`	Adds per-variable buffers to record the constraint index and coefficient that imply finite bounds for formerly-free variables; populates `presolve_info.bounded_free_variables` after bound selection.
Uncrush dual correction for bounded free variables `cpp/src/dual_simplex/presolve.cpp`	Extends `uncrush_solution` to accept the original problem and applies a post-solve loop: for each recorded entry compute `du = w_j / coefficient`, update `input_y[constraint] += du`, and adjust `input_z` via the original A matrix; updates explicit template instantiation.
Solver integration of uncrush with original problem `cpp/src/dual_simplex/solve.cpp`	Passes `original_lp` into `uncrush_solution()` in both advanced-basis and barrier solve paths.
Test infrastructure and free-variable dual correction validation `cpp/tests/dual_simplex/unit_tests/solve.cpp`, `cpp/tests/dual_simplex/unit_tests/solve_barrier.cu`	Adds logger initialization in several tests and a new barrier test `min_x_squared_free_variable_dual_correction` that loads an MPS, runs PDLP, prints solution vectors, and asserts expected primal/dual/reduced-cost values.

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ 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%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: fixing dual variable and reduced-cost computation when implied bounds are added to free variables.
Description check	✅ Passed	The description clearly explains the bug, the mathematical transformation applied, and references the fixed issue `#1119`.
Linked Issues check	✅ Passed	The PR directly addresses issue `#1119` by fixing dual/reduced-cost computation for QP problems when implied bounds are added to free variables, which resolves the factor-of-2 bug in constraint duals.
Out of Scope Changes check	✅ Passed	All changes are in scope: header/implementation updates for presolve tracking, dual correction logic in uncrush, solver call updates, and test additions validating the fix.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cpp/src/dual_simplex/presolve.cpp`:
- Around line 1758-1759: The dual correction unconditionally computes du = w_j /
bfv.coefficient which can produce Inf/NaN when bfv.coefficient is zero or
near-zero; in the block around du, guard the division by checking
fabs(bfv.coefficient) against a small epsilon (e.g. machine or
domain-appropriate epsilon) and only compute and apply input_y[bfv.constraint]
+= du when the coefficient is safe; if the coefficient is too small, skip the
update or handle it deterministically (clamp, set du = 0, or record/log the
event) to avoid division-by-zero and maintain numerical stability in the dual
correction path.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f10782e8-e45e-4214-85b0-0c7d011d8f83

📥 Commits

Reviewing files that changed from the base of the PR and between 6f99a42 and 23789ff.

📒 Files selected for processing (5)

cpp/src/dual_simplex/presolve.cpp
cpp/src/dual_simplex/presolve.hpp
cpp/src/dual_simplex/solve.cpp
cpp/tests/dual_simplex/unit_tests/solve.cpp
cpp/tests/dual_simplex/unit_tests/solve_barrier.cu

…fixes_2

chris-maes · 2026-05-19T16:37:21Z

/ok to test e8635f2

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cpp/src/dual_simplex/presolve.cpp`:
- Around line 991-1007: The code records bounded_free_variables in
current_free_variables order which breaks the postsolve "uncrush" replay because
later implied bounds can depend on earlier ones; change the recording so the
discovery/dependency order is preserved and ensure the replay applies fixes in
reverse of that discovery order (or mark and later reverse
presolve_info.bounded_free_variables before use). Specifically: when populating
presolve_info.bounded_free_variables in the loop over current_free_variables,
append entries in the actual implied-bound discovery order and then update the
uncrush/replay logic (the code that uses presolve_info.bounded_free_variables to
subtract original rows from input_z) to iterate the vector in reverse (or
reverse the vector once after filling) so previously corrected free variables
remain at zero.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: a4ecc2e0-7143-4aee-ab2c-e9a632961f4e

📥 Commits

Reviewing files that changed from the base of the PR and between 23789ff and e8635f2.

⛔ Files ignored due to path filters (1)

datasets/quadratic_programming/min_x_squared.mps is excluded by !**/*.mps

📒 Files selected for processing (3)

cpp/src/dual_simplex/presolve.cpp
cpp/src/dual_simplex/presolve.hpp
cpp/src/dual_simplex/solve.cpp

🚧 Files skipped from review as they are similar to previous changes (2)

cpp/src/dual_simplex/solve.cpp
cpp/src/dual_simplex/presolve.hpp

chris-maes · 2026-05-19T21:52:39Z

/ok to test e34a62a

…fixes_2

chris-maes · 2026-05-20T01:58:42Z

/ok to test 594b890

coderabbitai

🧹 Nitpick comments (1)

cpp/src/dual_simplex/presolve.cpp (1)
1784-1793: ⚖️ Poor tradeoff

Consider using row-wise access for the dual correction loop.

The current implementation iterates over all columns and performs a linear search within each column to find the constraint row. For large problems with many bounded free variables, this O(n × avg_col_nnz) cost per variable could be noticeable. Using a CSR representation or pre-building a row-index lookup would reduce this to O(row_nnz) per variable.

This is not blocking since the number of bounded free variables is typically small, and the fix is algorithmically correct.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/src/dual_simplex/presolve.cpp` around lines 1784 - 1793, The dual
correction loop over columns is doing repeated column scans to find entries for
bfv.constraint (see the loop using i_t j, A.col_start/col_end, A.i and A.x) and
updates input_z by subtracting A.x[p]*du; replace this by using row-wise access:
either iterate over a CSR-style row view for bfv.constraint (row_start/row_end
and arrays of column indices/values) or prebuild a lookup from row -> list of
(col, value) before the bounded-free variable loop, then for each bfv use that
row's nonzeros to apply input_z[col] -= value * du; this removes the per-column
linear search and keeps the same update to input_z and use of du.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@cpp/src/dual_simplex/presolve.cpp`:
- Around line 1784-1793: The dual correction loop over columns is doing repeated
column scans to find entries for bfv.constraint (see the loop using i_t j,
A.col_start/col_end, A.i and A.x) and updates input_z by subtracting A.x[p]*du;
replace this by using row-wise access: either iterate over a CSR-style row view
for bfv.constraint (row_start/row_end and arrays of column indices/values) or
prebuild a lookup from row -> list of (col, value) before the bounded-free
variable loop, then for each bfv use that row's nonzeros to apply input_z[col]
-= value * du; this removes the per-column linear search and keeps the same
update to input_z and use of du.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f60b25cc-d8a7-40ce-8060-af2f813da2e7

📥 Commits

Reviewing files that changed from the base of the PR and between e8635f2 and 594b890.

📒 Files selected for processing (1)

cpp/src/dual_simplex/presolve.cpp

mlubin

Didn't double check the math.

chris-maes · 2026-05-20T16:21:44Z

/merge

chris-maes · 2026-05-20T21:04:33Z

/ok to test b21b1bc

rapids-bot · 2026-05-20T22:12:03Z

This PR's base branch has been changed since the last /merge command. Please issue the command again to confirm the merge with the new base branch.

chris-maes · 2026-05-20T22:13:29Z

/merge

chris-maes requested a review from a team as a code owner May 18, 2026 21:12

chris-maes requested review from aliceb-nv and mlubin May 18, 2026 21:12

chris-maes mentioned this pull request May 18, 2026

Fix a bug in dual variables and reduced costs when we add implied bounds on free variables #1133

Closed

chris-maes self-assigned this May 18, 2026

chris-maes added bug Something isn't working non-breaking Introduces a non-breaking change labels May 18, 2026

chris-maes added this to the 26.06 milestone May 18, 2026

Add missing test file

06eb7d2

chris-maes requested a review from a team as a code owner May 18, 2026 21:14

chris-maes requested a review from rgsl888prabhu May 18, 2026 21:14

coderabbitai Bot reviewed May 18, 2026

View reviewed changes

Comment thread cpp/src/dual_simplex/presolve.cpp

rgsl888prabhu approved these changes May 18, 2026

View reviewed changes

Merge branch 'main' into dual_postsolve_fixes_2

bc59d72

anandhkb added the P0 label May 19, 2026

Merge branch 'main' into dual_postsolve_fixes_2

bb56519

rg20 approved these changes May 19, 2026

View reviewed changes

Merge remote-tracking branch 'cuopt-nvidia/main' into dual_postsolve_…

e8635f2

…fixes_2

coderabbitai Bot reviewed May 19, 2026

View reviewed changes

Comment thread cpp/src/dual_simplex/presolve.cpp

Merge branch 'main' into dual_postsolve_fixes_2

e34a62a

chris-maes added 2 commits May 19, 2026 18:56

Traverse in reverse order. Add more details in comment

309bc5d

Merge remote-tracking branch 'cuopt-nvidia/main' into dual_postsolve_…

594b890

…fixes_2

coderabbitai Bot reviewed May 20, 2026

View reviewed changes

mlubin approved these changes May 20, 2026

View reviewed changes

Comment thread cpp/tests/dual_simplex/unit_tests/solve_barrier.cu

Merge branch 'main' into dual_postsolve_fixes_2

7ea9991

rgsl888prabhu added the do not merge Do not merge if this flag is set label May 20, 2026

rgsl888prabhu changed the base branch from main to release/26.06 May 20, 2026 17:29

chris-maes removed the do not merge Do not merge if this flag is set label May 20, 2026

Merge branch 'release/26.06' into dual_postsolve_fixes_2

b21b1bc

rapids-bot Bot merged commit 0e40d37 into NVIDIA:release/26.06 May 20, 2026
97 checks passed

chris-maes deleted the dual_postsolve_fixes_2 branch May 20, 2026 22:13

This was referenced May 21, 2026

Extend the barrier solver to SOCP problems #1051

Closed

Main merge release/26.06 #1270

Closed

Conversation

chris-maes commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

copy-pr-bot Bot commented May 18, 2026

Uh oh!

chris-maes commented May 18, 2026

Uh oh!

chris-maes commented May 18, 2026

Uh oh!

coderabbitai Bot commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chris-maes commented May 19, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chris-maes commented May 19, 2026

Uh oh!

chris-maes commented May 20, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

mlubin left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chris-maes commented May 20, 2026

Uh oh!

chris-maes commented May 20, 2026

Uh oh!

rapids-bot Bot commented May 20, 2026

Uh oh!

chris-maes commented May 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

chris-maes commented May 18, 2026 •

edited

Loading

coderabbitai Bot commented May 18, 2026 •

edited

Loading

mlubin left a comment •

edited

Loading