Skip to content

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

Open
chris-maes wants to merge 3 commits into
NVIDIA:mainfrom
chris-maes:dual_postsolve_fixes_2
Open

Fix a bug in dual variables and reduced costs when we add implied bounds on free variables#1237
chris-maes wants to merge 3 commits into
NVIDIA:mainfrom
chris-maes:dual_postsolve_fixes_2

Conversation

@chris-maes
Copy link
Copy Markdown
Contributor

@chris-maes chris-maes commented May 18, 2026

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.
@chris-maes chris-maes requested a review from a team as a code owner May 18, 2026 21:12
@chris-maes chris-maes requested review from aliceb-nv and mlubin May 18, 2026 21:12
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 18, 2026

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 chris-maes self-assigned this May 18, 2026
@chris-maes chris-maes added bug Something isn't working non-breaking Introduces a non-breaking change labels May 18, 2026
@chris-maes chris-maes added this to the 26.06 milestone May 18, 2026
@chris-maes
Copy link
Copy Markdown
Contributor Author

/ok to test 23789ff

@chris-maes chris-maes requested a review from a team as a code owner May 18, 2026 21:14
@chris-maes chris-maes requested a review from rgsl888prabhu May 18, 2026 21:14
@chris-maes
Copy link
Copy Markdown
Contributor Author

/ok to test 06eb7d2

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 18, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR extends presolve to track which constraints imply finite bounds on formerly-free variables, then applies a post-solve dual correction during solution uncrushing. Presolve records constraint indices and coefficients for each bounded variable, and uncrush uses these to adjust dual variables and reduced costs via the original constraint matrix.

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> to store variable index, originating constraint index, and coefficient. Extends presolve_info_t<i_t, f_t> with bounded_free_variables member. Updates uncrush_solution declaration to accept original_problem parameter.
Presolve tracking of bound-originating constraints
cpp/src/dual_simplex/presolve.cpp
Adds per-variable buffers in presolve() to record constraint and coefficient for each finite bound acquired by a free variable. After bound selection, populates presolve_info.bounded_free_variables with variable, constraint, and coefficient tuples for each bounded free variable.
Uncrush dual correction for bounded free variables
cpp/src/dual_simplex/presolve.cpp
Updates uncrush_solution() to accept original problem matrix. Implements post-solve correction block that computes du = w_j / coefficient for each bounded free variable, updates dual variable input_y[constraint] += du, and adjusts reduced costs via original_problem.A. Updates template instantiation signature.
Solver integration of uncrush with original problem
cpp/src/dual_simplex/solve.cpp
Both solve_linear_program_with_advanced_basis and solve_linear_program_with_barrier now pass original_lp when calling uncrush_solution().
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
Existing tests initialize cuopt::init_logger_t for diagnostics. New TEST(barrier, min_x_squared_free_variable_dual_correction) loads an MPS dataset, solves via PDLP, and validates primal solution, dual solution, and reduced-cost correctness under the dual correction mechanism.

🎯 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 summarizes the main change: fixing a bug related to dual variables and reduced costs when implied bounds are added on free variables.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description accurately describes the mathematical problem being solved and explains the dual correction strategy for free variables with implied bounds.

✏️ 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.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread cpp/src/dual_simplex/presolve.cpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] QP constraint duals returned by cuOpt are off by a factor of 2

2 participants