Fix the bug detecting nonconvex quadratic constraints in the general …

[yuwenchen95]

Description

Cross-only indefinite quadratic constraints (e.g. 2·x₀·x₁ ≤ 0.5 with H = [[0,2],[2,0]]) previously passed convexity checks on the general SOC conversion path. Diagonal LDLT returned rank = 0 without INDEFINITE_MATRIX_RETURN, and a degenerate r = 0 SOC lift silently dropped the quadratic term, allowing wrong solutions to be reported as Optimal (e.g. obj=2 at (1,1) instead of rejecting the problem).

Fixes:

• right_looking_ldlt: return INDEFINITE_MATRIX_RETURN when factorization stalls at rank = 0 on a nonzero matrix.
• translate_soc.hpp (general path): cuopt_expects(rank >= 1, …) before building the SOC lift.

Adds LDLT and SOC-conversion unit tests for the cross-only indefinite case.

Issue

closes#1434

Checklist

• I am familiar with the Contributing Guidelines.
• Testing
  • New or existing tests cover these changes
  • Added tests
  • Created an issue to follow-up
  • NA
• Documentation
  • The documentation is up to date with these changes
  • Added new documentation
  • NA

Test plan

• ./build.sh libcuopt with conda env active, then run built/installed cuopt_cli against $CONDA_PREFIX/lib/libcuopt.so
• C++ GTest:
  • ./cpp/build/tests/dual_simplex/DUAL_SIMPLEX_TEST --gtest_filter='right_looking_ldlt.indefinite_cross_only_2x2'
  • ./cpp/build/tests/socp/SOCP_TEST --gtest_filter='general_quadratic.rejects_cross_only_indefinite'
  • ./cpp/build/tests/socp/SOCP_TEST --gtest_filter='general_quadratic.rejects_non_convex'
• Manual: datasets/qcqp/issue_1434_nonconvex.lp → ValidationError (not Optimal obj=2)

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The PR updates cross-only indefinite quadratic handling in LDLT-based SOC conversion, adds runtime rejection for degenerate factorization results, routes MPS problem population through a helper, and extends tests for quadratic-constraint preservation and solver rejection.

Changes

Cross-only indefinite QCQP handling

Layer / File(s)	Summary
LDLT pivot-failure signaling cpp/src/dual_simplex/right_looking_lu.hpp, cpp/src/dual_simplex/right_looking_lu.cpp	right_looking_ldlt now records input_nnz, returns INDEFINITE_MATRIX_RETURN when pivot search fails on a nonzero matrix before any pivots are chosen, and updates the return-code comment. The unused #include <cstdio> is removed.
Quadratic-constraint population and translation cpp/src/pdlp/solve.cu, cpp/src/pdlp/translate.hpp, cpp/src/barrier/translate_soc.hpp	mps_data_model_to_optimization_problem now calls populate_from_mps_data_model. The user-problem translation path handles m == 0, converts quadratic constraints before CSR column compression, and convert_quadratic_constraints_to_second_order_cones now rejects rank-0 LDLT results with cuopt_expects(rank >= 1, ...).
Conversion and rejection tests cpp/tests/linear_programming/unit_tests/solution_interface_test.cu, cpp/tests/socp/general_quadratic_test.cu, cpp/tests/dual_simplex/unit_tests/right_looking_ldlt.cpp	Tests verify quadratic constraints survive LP/MPS conversion and that a cross-only indefinite quadratic constraint is rejected with ValidationError, plus a direct LDLT regression for the 2x2 zero-diagonal case.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

• [[BUG] incorrect "optimal" solution on nonconvex qcqp #1434](linked issue) — Matches the nonconvex cross-only QCQP rejection path described in this PR.

Suggested reviewers

• Iroy30
• ramakrishnan-nv

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 54.55% 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 clearly names the main fix: rejecting nonconvex quadratic constraints in the general path.
Description check	✅ Passed	The description matches the changes and explains the bug, fix, and tests.
Linked Issues check	✅ Passed	The patch rejects the reported nonconvex QCQP instead of reporting optimal, and adds tests for the cross-only case [#1434].
Out of Scope Changes check	✅ Passed	The extra translation and test updates support the same quadratic-constraint fix and do not appear unrelated.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[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/right_looking_lu.cpp`:
- Around line 1686-1687: Add a new method `has_remaining_numerically_nonzero` to
the `symmetric_trailing_matrix_t` class that checks whether the trailing matrix
has any numerically nonzero elements by iterating through diagonal elements and
off-diagonal elements, comparing them against a numeric tolerance parameter.
Then, refine the indefinite return condition around line 1726 (in the 1723-1727
range) so that it returns indefinite not only when `pivots == 0`, but also when
the trailing matrix is still numerically nonzero after partial elimination,
ensuring mixed cases with one early pivot followed by remaining cross-only
indefinite blocks are properly handled.

🪄 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: 8a87302e-97ad-4c39-a947-ab3d6cfcd39e

📥 Commits

Reviewing files that changed from the base of the PR and between 03fe3fc and cedb27f.

📒 Files selected for processing (5)

• cpp/src/barrier/translate_soc.hpp
• cpp/src/dual_simplex/right_looking_lu.cpp
• cpp/src/dual_simplex/right_looking_lu.hpp
• cpp/tests/dual_simplex/unit_tests/right_looking_ldlt.cpp
• cpp/tests/socp/general_quadratic_test.cu

[coderabbitai]

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Handle no-pivot stalls after partial elimination as indefinite when the trailing matrix is still nonzero.

At Line 1726, the indefinite return is gated by pivots == 0, so mixed cases can still slip through (e.g., one valid early pivot, then a remaining cross-only indefinite block). That returns a positive rank and can still let non-convex QCQP pass conversion.

🛠️ Suggested fix

- const i_t n = A.n;- const i_t input_nnz = A.nnz();+ const i_t n = A.n;
...
- if (pivot_p == -1) {- // No acceptable diagonal pivot. Zero matrix is rank 0; nonzero cross-only indefinite- // matrices (e.g. [[0, a]; [a, 0]]) also stall here and must not be treated as PSD.- if (pivots == 0 && input_nnz > 0) { return INDEFINITE_MATRIX_RETURN; }- break;- }+ if (pivot_p == -1) {+ // If any significant entry remains in the trailing matrix, matrix is not PSD.+ if (trailing_matrix.has_remaining_numerically_nonzero(pivot_tol)) {+ return INDEFINITE_MATRIX_RETURN;+ }+ break; // true rank-deficient zero remainder+ }

// Add to symmetric_trailing_matrix_tboolhas_remaining_numerically_nonzero(f_t nz_tol) const
{
for (i_t j = 0; j < n_; ++j) {
if (std::abs(diag_[j]) >= nz_tol) { returntrue; }
for (i_t p = col_start_[j]; p < col_end_[j]; ++p) {
if (std::abs(c_x_[p]) >= nz_tol) { returntrue; }
}
}
returnfalse;
}

As per coding guidelines, solver reviews should catch degenerate/numerical transformation-path correctness gaps.

Also applies to: 1723-1727

🤖 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/right_looking_lu.cpp` around lines 1686 - 1687, Add a
new method `has_remaining_numerically_nonzero` to the
`symmetric_trailing_matrix_t` class that checks whether the trailing matrix has
any numerically nonzero elements by iterating through diagonal elements and
off-diagonal elements, comparing them against a numeric tolerance parameter.
Then, refine the indefinite return condition around line 1726 (in the 1723-1727
range) so that it returns indefinite not only when `pivots == 0`, but also when
the trailing matrix is still numerically nonzero after partial elimination,
ensuring mixed cases with one early pivot followed by remaining cross-only
indefinite blocks are properly handled.

Source: Coding guidelines

[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.

[rg20]

How long does the conversion take? I think this loop can be parallelized to speed this up.

You can do it as another PR though.

[yuwenchen95]

It really depends on the problem class. Usually it takes less than 1%, but there is a problem class that may take up to 8% of total solve time.

I would do it later in another PR aiming efficiency improvement of setup phase for QCQPs.

[yuwenchen95]

/ok to test e863795

[github-actions]

CI Test Summary

✅ All 31 test job(s) passed.

[mlubin]

We should very rarely have code that fully qualifies nanes with cuopt::mathematical_optimization

[mlubin]

Is this the right file for an end-to-end solve test if cuopt/mathematical_optimization/solve.hpp was not previously included?

[yuwenchen95]

I think it's ok since the file is solely for tests related to general quadratic convex constraints.

[mlubin]

We should follow the pattern in the surrounding context and write the test with solve_linear_program_with_barrier instead of solve_lp.

[mlubin]

Why is user_problem.A initialized above if it's set here?

[yuwenchen95]

We need csr_A for the use in convert_quadratic_constraints_to_second_order_cones. The current input pipeline has several transformation between csc and csr format for the efficiency of each block.

[yuwenchen95]

/merge