Add support for general convex quadratic constraints (x^T Q x + c^T x… by chris-maes · Pull Request #1361 · NVIDIA/cuopt

chris-maes · 2026-06-01T22:44:40Z

Implement sparse LDL^T factorization with symmetric Markowitz pivoting and use it to translate general convex quadratic constraints into second-order cone form for the barrier solver.

Sparse LDL^T factorization: Add symmetric_trailing_matrix_t class that stores the lower triangle of a symmetric matrix with column storage, row index, diagonal cache, and a single degree structure exploiting symmetry. Add right_looking_ldlt function that computes PAP^T = LDL^T for symmetric PSD matrices using minimum- degree pivot selection with largest diagonal tiebreaker within each degree level. Returns an error code for indefinite (non-PSD) input.

General quadratic constraint translation: Add fallback path in translate_soc.hpp when Q doesn't fit existing special-case SOC patterns (non-uniform diagonals, multiple off-diagonals, nonzero RHS, etc.). Symmetrize Q from COO into H using a dense accumulator on the local variable subset. Factorize H via right_looking_ldlt and reject indefinite (non-convex) Q with a validation error. Construct a standard SOC with linking rows derived from the LDL^T factors. Properly invalidate affine-head allocations for constraints handled by the general path.

Add unit tests for LDL^T: diagonal, dense PD, rank-1 PSD, rank-2 PSD, zero matrix, scalar, 5x5 PD, rank-3 5x5 PSD, identity, graph Laplacian, symmetrized from unsymmetric, 10x10 sparse single entry, indefinite 2x2, indefinite 4x4.

Add unit tests for general quadratic translation:
dense PD with equality constraint, non-convex rejection, rank-deficient PSD, inequality constraint alongside quadratic constraint, least-squares with b in range of A (optimal residual zero), least-squares with b not in range of A (optimal residual positive).

… <= alpha) Implement sparse LDL^T factorization with symmetric Markowitz pivoting and use it to translate general convex quadratic constraints into second-order cone form for the barrier solver. Sparse LDL^T factorization: Add symmetric_trailing_matrix_t class that stores the lower triangle of a symmetric matrix with column storage, row index, diagonal cache, and a single degree structure exploiting symmetry. Add right_looking_ldlt function that computes PAP^T = LDL^T for symmetric PSD matrices using minimum- degree pivot selection with largest diagonal tiebreaker within each degree level. Returns an error code for indefinite (non-PSD) input. General quadratic constraint translation: Add fallback path in translate_soc.hpp when Q doesn't fit existing special-case SOC patterns (non-uniform diagonals, multiple off-diagonals, nonzero RHS, etc.). Symmetrize Q from COO into H using a dense accumulator on the local variable subset. Factorize H via right_looking_ldlt and reject indefinite (non-convex) Q with a validation error. Construct a standard SOC with linking rows derived from the LDL^T factors. Properly invalidate affine-head allocations for constraints handled by the general path. Add unit tests for LDL^T: diagonal, dense PD, rank-1 PSD, rank-2 PSD, zero matrix, scalar, 5x5 PD, rank-3 5x5 PSD, identity, graph Laplacian, symmetrized from unsymmetric, 10x10 sparse single entry, indefinite 2x2, indefinite 4x4. Add unit tests for general quadratic translation: dense PD with equality constraint, non-convex rejection, rank-deficient PSD, inequality constraint alongside quadratic constraint, least-squares with b in range of A (optimal residual zero), least-squares with b not in range of A (optimal residual positive).

copy-pr-bot · 2026-06-01T22:44:44Z

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.

…orm_general_quadratic_constraints

coderabbitai · 2026-06-01T22:46:05Z

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

Adds a right-looking sparse symmetric LDLᵀ factorization (Markowitz pivoting) and integrates it into translate_quadratic to convert general convex quadratic constraints into SOCs; includes API, implementation, unit tests, SOCP integration tests, and barrier-layout dimension fixes.

Changes

Symmetric LDLT Factorization and General Quadratic Constraint Support

Layer / File(s)	Summary
LDLT API and Return Code `cpp/src/dual_simplex/right_looking_lu.hpp`, `cpp/src/dual_simplex/types.hpp`	Declares `right_looking_ldlt` function signature with documented return semantics (success, indefinite, halt, timeout) and introduces `INDEFINITE_MATRIX_RETURN` sentinel (-4).
LDLT Factorization Implementation `cpp/src/dual_simplex/right_looking_lu.cpp`	Implements symmetric trailing-matrix data structure with Markowitz pivot selection, symmetric Schur-complement updates on the lower triangle, and the main right-looking LDLᵀ factorization algorithm; adds template instantiation for `int`/`double`.
LDLT Factorization Unit Tests `cpp/tests/dual_simplex/CMakeLists.txt`, `cpp/tests/dual_simplex/unit_tests/right_looking_ldlt.cpp`	Configures LDLT test suite, provides helpers to convert dense symmetric matrices to CSC and verify factorizations, and covers 15 test cases: small and large PD/PSD matrices, rank-deficient cases, zero matrices, sparse single-entry, graph Laplacians, and indefinite rejection tests.
Quadratic Constraint Pattern Detection and Routing `cpp/src/barrier/translate_soc.hpp` (headers, Q processing, decision logic)	Adds dual-simplex headers, replaces strict validation with flags for duplicate rows and near-zero diagonals, computes a `use_general_path` decision gate based on Q structure properties, and moves RHS zero-enforcement to specialized patterns only.
General Convex Quadratic Conversion `cpp/src/barrier/translate_soc.hpp` (general path, dimension updates, problem expansion)	Implements general quadratic handling: constructs an H matrix from Q, factorizes via LDLᵀ, rejects indefinite cases, registers an SOC with rank-dependent dimension, extends the problem with new variables and equality rows linking auxiliaries to the original model, invalidates incompatible affine auxiliaries, and updates CSR dimensions using max-preservation logic.
Barrier Layout & Dimension Finalization `cpp/src/barrier/translate_soc.hpp`	When appending affine auxiliaries, updates `csr_A.n = max(csr_A.n, n_aug)` and sets barrier problem size `n_prob` from post-expansion `csr_A.n`.
SOCP Integration and Validation Tests `cpp/tests/socp/CMakeLists.txt`, `cpp/tests/socp/general_quadratic_test.cu`	Configures SOCP test target, initializes cuBLAS/cuSPARSE device modes, and provides multiple integration scenarios: 2×2 dense PD solve with barrier solver, indefinite rejection, rank-deficient PSD with cone dimension assertion, inequality constraint combinations, least-squares in-/out-of-range cases, and SOC head acceptance/rejection tests.

🎯 4 (Complex) | ⏱️ ~60 minutes

rgsl888prabhu

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 79.49% 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: adding support for general convex quadratic constraints, which is the primary objective of the PR.
Description check	✅ Passed	The description provides detailed information about implementing sparse LDL^T factorization and translating general convex quadratic constraints, directly related to the changeset.
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.

_{✏️ 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: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

cpp/src/barrier/translate_soc.hpp (1)
775-780: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win
Avoid shrinking variable arrays after general-path expansion.
n_aug is based on n_with_affine_aux; in mixed cases this can be smaller than current csr_A.n (already expanded by the general path). Resizing user_problem.* to n_aug truncates variable metadata and creates dimension mismatches.
🔧 Suggested fix
-const i_t n_aug = n_with_affine_aux;+const i_t n_aug = std::max(csr_A.n, n_with_affine_aux);
user_problem.objective.resize(n_aug, 0);
user_problem.lower.resize(n_aug, -inf);
user_problem.upper.resize(n_aug, inf);
user_problem.var_types.resize(
n_aug, cuopt::linear_programming::dual_simplex::variable_type_t::CONTINUOUS);
-csr_A.n = std::max(csr_A.n, n_aug);+csr_A.n = n_aug;
As per coding guidelines, transformation integrity checks should ensure counts and dimensions remain consistent after recomputation and expansion.
Also applies to: 796-796
🤖 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/barrier/translate_soc.hpp` around lines 775 - 780, The resize calls
on user_problem.* (user_problem.objective, lower, upper, var_types, col_names)
using n_aug can shrink these arrays and truncate metadata because n_aug (from
n_with_affine_aux) may be smaller than the already-expanded csr_A.n; instead,
ensure you never shrink existing variable arrays — only grow them to at least
max(current_size, n_aug) or validate that n_aug >= csr_A.n before resizing.
Update the logic around the expansion (referencing n_aug, csr_A.n, and
user_problem.*) to use a safe_resize/grow pattern or a guard that preserves
existing elements and only extends arrays when needed, and add an assertion or
consistency check to validate final counts after general-path expansion.

🧹 Nitpick comments (1)

cpp/src/dual_simplex/types.hpp (1)
26-27: ⚡ Quick win
Use project macro prefix for the new return-code constant.
Please rename INDEFINITE_MATRIX_RETURN to a CUOPT_... macro (or add a prefixed alias and migrate callers) to match project macro conventions in headers.
As per coding guidelines, “Project macros must use SCREAMING_SNAKE_CASE with CUOPT_ prefix.”
🤖 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/types.hpp` around lines 26 - 27, The macro
INDEFINITE_MATRIX_RETURN violates project convention; rename it to a
CUOPT-prefixed macro (e.g., CUOPT_INDEFINITE_MATRIX_RETURN) or add a prefixed
alias and migrate callers: update the definition in types.hpp to define
CUOPT_INDEFINITE_MATRIX_RETURN -4 and either replace all uses of
INDEFINITE_MATRIX_RETURN with CUOPT_INDEFINITE_MATRIX_RETURN or leave a
transitional `#define` INDEFINITE_MATRIX_RETURN CUOPT_INDEFINITE_MATRIX_RETURN and
update call sites (functions/types referencing INDEFINITE_MATRIX_RETURN) to the
new CUOPT_ name to comply with SCREAMING_SNAKE_CASE CUOPT_ prefix policy.

🤖 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/barrier/translate_soc.hpp`:
- Around line 578-590: The code currently uses exact comparisons (val != f_t(0))
when counting and inserting nonzeros into H_csc (variables/functions: H_dense,
h_nnz, H_csc, f_t, i_t), which preserves floating-point noise as structural
nonzeros; change both the counting loop that computes h_nnz and the insertion
loop that fills H_csc to use a tolerance test instead (e.g., fabs(val) > tol).
Compute tol once before these loops using a sensible rule such as tol = C *
std::numeric_limits<f_t>::epsilon() * max(1, max_abs_entry_of_H_dense) or a
scaled row/column norm, and use that same tol consistently so h_nnz matches the
actual inserted entries. Ensure include/using of std::fabs and numeric_limits
where needed and update logic that relies on h_nnz to remain consistent.
- Around line 607-617: The code only checks for INDEFINITE_MATRIX_RETURN after
calling dual_simplex::right_looking_ldlt and then uses rank; change this to
first test the return value from right_looking_ldlt for all non-success/negative
codes (e.g., any return < 0) before using rank, and handle each case
appropriately (for INDEFINITE_MATRIX_RETURN keep the existing cuopt_expects
check; for other negative/error returns log/raise a suitable error or propagate
using cuopt_expects or an InternalError), referencing the call to
dual_simplex::right_looking_ldlt and the variables H_csc, ldlt_settings,
ldlt_start, ldlt_perm, L_factor, D_factor, ldlt_work and the local 'rank' so
that no path uses rank when right_looking_ldlt returned an error.
- Around line 607-609: The call to dual_simplex::right_looking_ldlt uses a
hardcoded tolerance f_t(1e-12); replace this with the solver's active LDLᵀ
tolerance scale used elsewhere in the conversion/settings path so the tolerance
follows the solver scaling policy. Locate the variable or accessor that exposes
the active tolerance scale (the same one used when configuring
conversion/settings) and pass that value instead of f_t(1e-12) into
dual_simplex::right_looking_ldlt (keeping H_csc, ldlt_settings, ldlt_start,
ldlt_perm, L_factor, D_factor, ldlt_work as-is). Ensure the chosen symbol type
matches f_t so compilation and numeric semantics remain correct.
In `@cpp/src/dual_simplex/right_looking_lu.cpp`:
- Around line 1726-1728: Replace the strict negative pivot check with a
tolerance-aware test: instead of if (pivot_val < 0) return
INDEFINITE_MATRIX_RETURN; compare pivot_val against a solver-appropriate
tolerance (e.g., if (pivot_val < -pivot_tol) return INDEFINITE_MATRIX_RETURN; or
scale pivot_tol by local row/column norms if a scaled tolerance facility exists)
so that small negative values due to numerical noise are not treated as
indefinite; use the existing solver tolerance variable or introduce pivot_tol
nearby and document its provenance, and keep the INDEFINITE_MATRIX_RETURN path
unchanged.
In `@cpp/tests/socp/general_quadratic_test.cu`:
- Line 227: The test rank_deficient_psd_solve sets objective = {1.0, 0.0} but
intends to minimize x0+x1, which is why the expected optimum/assert is wrong and
the loose tolerance hides the bug; change the objective initialization to {1.0,
1.0} so it actually optimizes x0+x1, then update the corresponding expected
optimum assertions to -2 with a tight numeric tolerance (e.g., 1e-6) in the same
test (and the second occurrence mentioned) to catch regressions.
---
Outside diff comments:
In `@cpp/src/barrier/translate_soc.hpp`:
- Around line 775-780: The resize calls on user_problem.*
(user_problem.objective, lower, upper, var_types, col_names) using n_aug can
shrink these arrays and truncate metadata because n_aug (from n_with_affine_aux)
may be smaller than the already-expanded csr_A.n; instead, ensure you never
shrink existing variable arrays — only grow them to at least max(current_size,
n_aug) or validate that n_aug >= csr_A.n before resizing. Update the logic
around the expansion (referencing n_aug, csr_A.n, and user_problem.*) to use a
safe_resize/grow pattern or a guard that preserves existing elements and only
extends arrays when needed, and add an assertion or consistency check to
validate final counts after general-path expansion.
---
Nitpick comments:
In `@cpp/src/dual_simplex/types.hpp`:
- Around line 26-27: The macro INDEFINITE_MATRIX_RETURN violates project
convention; rename it to a CUOPT-prefixed macro (e.g.,
CUOPT_INDEFINITE_MATRIX_RETURN) or add a prefixed alias and migrate callers:
update the definition in types.hpp to define CUOPT_INDEFINITE_MATRIX_RETURN -4
and either replace all uses of INDEFINITE_MATRIX_RETURN with
CUOPT_INDEFINITE_MATRIX_RETURN or leave a transitional `#define`
INDEFINITE_MATRIX_RETURN CUOPT_INDEFINITE_MATRIX_RETURN and update call sites
(functions/types referencing INDEFINITE_MATRIX_RETURN) to the new CUOPT_ name to
comply with SCREAMING_SNAKE_CASE CUOPT_ prefix policy.

🪄 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: 63a2946d-15ae-4706-bd32-54af6d2bf8e1

📥 Commits

Reviewing files that changed from the base of the PR and between 5fc5509 and de8cd97.

📒 Files selected for processing (8)

cpp/src/barrier/translate_soc.hpp
cpp/src/dual_simplex/right_looking_lu.cpp
cpp/src/dual_simplex/right_looking_lu.hpp
cpp/src/dual_simplex/types.hpp
cpp/tests/dual_simplex/CMakeLists.txt
cpp/tests/dual_simplex/unit_tests/right_looking_ldlt.cpp
cpp/tests/socp/CMakeLists.txt
cpp/tests/socp/general_quadratic_test.cu

coderabbitai · 2026-06-01T22:54:20Z

+ // Check for indefiniteness: a negative pivot means the matrix is not PSD
+ if (pivot_val < 0) { return INDEFINITE_MATRIX_RETURN; }
+


⚠️ Potential issue | 🟠 Major | ⚡ Quick win
Use tolerance-aware indefiniteness detection for pivot sign.
The strict check pivot_val < 0 can misclassify numerically noisy PSD pivots as indefinite. Compare against -pivot_tol (or a solver-scaled tolerance) before returning INDEFINITE_MATRIX_RETURN.
Suggested fix
- if (pivot_val < 0) { return INDEFINITE_MATRIX_RETURN; }+ if (pivot_val < -pivot_tol) { return INDEFINITE_MATRIX_RETURN; }
As per coding guidelines, avoid hardcoded/overly-strict pivot checks lacking solver-appropriate tolerance for LDLᵀ/PSD handling.
🤖 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 1726 - 1728, Replace the strict negative pivot check with a tolerance-aware test: instead of if (pivot_val < 0) return INDEFINITE_MATRIX_RETURN; compare pivot_val against a solver-appropriate tolerance (e.g., if (pivot_val < -pivot_tol) return INDEFINITE_MATRIX_RETURN; or scale pivot_tol by local row/column norms if a scaled tolerance facility exists) so that small negative values due to numerical noise are not treated as indefinite; use the existing solver tolerance variable or introduce pivot_tol nearby and document its provenance, and keep the INDEFINITE_MATRIX_RETURN path unchanged.

chris-maes · 2026-06-02T00:10:47Z

/ok to test 0e5869c

The standard SOC translation of -sx_head^2 + s||x_tail||^2 <= 0 into ||x_tail|| <= x_head is only valid when x_head >= 0. Without this constraint, the original inequality represents ||x_tail|| <= |x_head| which is non-convex. Add a validation check that the head variable has lower >= 0 before accepting the special-case SOC translation. If the head is free, reject with a clear error message indicating the constraint is non-convex. Unit tests: soc_head_nonneg_accepted verifies that x^2+y^2-t^2<=0 with t>=0 is accepted; soc_head_free_rejected verifies the same constraint with t free is rejected.

chris-maes · 2026-06-02T00:23:56Z

/ok to test 5663cd6

coderabbitai

🧹 Nitpick comments (1)

cpp/tests/socp/general_quadratic_test.cu (1)

607-620: 💤 Low value

Remove the overwritten dead setup and misleading comments.

Lines 611-615 are immediately overwritten by the reassignment at 618-620, so the writes to A.i[1]/A.x[1] are dead and the comments (dummy row for barrier: t <= 10, placeholder) describe a constraint that no longer exists (the effective constraint is x1 = 1). nz is also 2 while only one entry is used. This looks like leftover debugging cruft.

♻️ Proposed cleanup

- constexpr int m = 1;- constexpr int n = 3;- constexpr int nz = 2;+ constexpr int m = 1;+ constexpr int n = 3;+ constexpr int nz = 1;
@@
user_problem.A.nz_max = nz;
user_problem.A.reallocate(nz);
- user_problem.A.col_start = {0, 0, 0, 2};- user_problem.A.i[0] = 0;- user_problem.A.x[0] = 1.0; // dummy row for barrier: t <= 10- user_problem.A.i[1] = 0;- user_problem.A.x[1] = 0.0; // placeholder-- // Actually just use: x1 = 1 as a simple equality- user_problem.A.col_start = {0, 0, 1, 1};- user_problem.A.i[0] = 0;- user_problem.A.x[0] = 1.0;+ // Simple equality: x1 = 1+ user_problem.A.col_start = {0, 0, 1, 1};+ user_problem.A.i[0] = 0;+ user_problem.A.x[0] = 1.0;

🤖 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/tests/socp/general_quadratic_test.cu` around lines 607 - 620, Remove the
dead/overwritten setup and misleading comments for user_problem.A: delete the
initial assignments that set user_problem.A.col_start = {0,0,0,2} and the writes
to user_problem.A.i[1] and user_problem.A.x[1] and their comments, then ensure
the sparse matrix metadata matches the actual entries (set nz to 1 if only one
nonzero is used) and keep the final intended setup (user_problem.A.col_start =
{0,0,1,1} with user_problem.A.i[0]=0 and user_problem.A.x[0]=1.0) so the matrix,
nz and comments consistently represent the equality x1 = 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.
Nitpick comments:
In `@cpp/tests/socp/general_quadratic_test.cu`:
- Around line 607-620: Remove the dead/overwritten setup and misleading comments
for user_problem.A: delete the initial assignments that set
user_problem.A.col_start = {0,0,0,2} and the writes to user_problem.A.i[1] and
user_problem.A.x[1] and their comments, then ensure the sparse matrix metadata
matches the actual entries (set nz to 1 if only one nonzero is used) and keep
the final intended setup (user_problem.A.col_start = {0,0,1,1} with
user_problem.A.i[0]=0 and user_problem.A.x[0]=1.0) so the matrix, nz and
comments consistently represent the equality x1 = 1.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 28a73739-6669-449b-bcbd-6b8ac7f78e50

📥 Commits

Reviewing files that changed from the base of the PR and between 5d1a302 and 5663cd6.

📒 Files selected for processing (2)

cpp/src/barrier/translate_soc.hpp
cpp/tests/socp/general_quadratic_test.cu

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

cpp/src/barrier/translate_soc.hpp

…orm_general_quadratic_constraints

chris-maes · 2026-06-02T09:50:34Z

/ok to test 3046bd8

…ranslate when y >= 0, z >= 0

yuwenchen95 · 2026-06-02T08:16:46Z

+ }
+ }
+ }
+ const bool use_general_path = has_duplicate_rows || has_near_zero_diag || has_nonzero_rhs ||


We should throw an error once has_near_zero_diag is true, since positive semidefinite matrix requires all diagonals to be nonnegative.

near_zero is not negative right?

In a PSD matrix you are allowed to have diagonals that are exactly zero.

yuwenchen95 · 2026-06-02T08:53:29Z

+
+ if (offdiag_entries.empty()) {
+ if (!has_linear_part) {
+ if (pos_diag_rows.empty()) {


Do we need this branchingif (pos_diag_rows.empty()) here? This corresponds to −s*x^2 ≤0,rhs=0,s>0, which is not we expect for second-order cone.

This is part of the previous code. That is, if(pos_diag_rows.empty()) was part of the previous special case code.

yuwenchen95 · 2026-06-02T09:01:56Z

+ }
+ }
+ }
+ const bool use_general_path = has_duplicate_rows || has_near_zero_diag || has_nonzero_rhs ||


It's better to define use_fast_path since we have clear conditions for three special cases we want to address separately. Agents seem to generate additional branching for !use_general_path that doesn't belong to three special cases.

We are trying to do the fast path first, then the general path.

Can you be more specific about "additional branching that doesn't belong to three special cases"

yuwenchen95 · 2026-06-02T09:14:29Z

+ user_problem.var_types.assign(n, variable_type_t::CONTINUOUS);
+
+ // Build quadratic constraint: x^T [2 1; 1 2] x <= 1
+ // Q in COO (lower triangle stored per MPS convention):


Why do we use only lower triangle part here? I remember MPS requires explicit full symmetric matrix representation for quadratic cost in a quadratic constraint, e.g. x1*x2 and 'x2*x1'.

The factorization only takes in the lower triangular part. We use the factorization to determine if the matrix is PSD.

Oh sorry this in the test. Remember the C API and Python API are different from MPS. In C and Python we support unsymmetric matrices.

Ah I see. The comment about MPS storing the lower triangluar part is wrong. I will fix that.

yuwenchen95 · 2026-06-02T09:16:31Z

+ qc.constraint_row_name = "non_convex";
+ qc.constraint_row_type = 'L';
+ qc.rhs_value = 1.0;
+ qc.rows = {0, 1, 1};


The same concern about the full symmetric representation in MPS files as above.

See answer above.

yuwenchen95 · 2026-06-02T09:17:45Z

+ qc.constraint_row_name = "rank1_cone";
+ qc.constraint_row_type = 'L';
+ qc.rhs_value = 4.0;
+ qc.rows = {0, 1, 1};


The same concern as above.

See answer above.

yuwenchen95 · 2026-06-02T09:18:00Z

+ qc.constraint_row_name = "ellipse_ineq";
+ qc.constraint_row_type = 'L';
+ qc.rhs_value = 1.0;
+ qc.rows = {0, 1, 1};


The same concern as above.

See answer above

…c constraints

…orm_general_quadratic_constraints

chris-maes · 2026-06-02T18:29:22Z

/ok to test c7a9fda

…orm_general_quadratic_constraints

chris-maes · 2026-06-02T18:37:51Z

/ok to test c72d0cc

ramakrishnap-nv · 2026-06-02T19:05:54Z

/ok to test c287553

chris-maes · 2026-06-02T20:51:53Z

/ok to test e34b93c

chris-maes · 2026-06-02T22:32:12Z

/merge

Adds general_quadratic_example.{c,py} demonstrating a general convex quadratic constraint (2x^2 + 2xy + 2y^2 <= 6): nonzero rhs and a cross term, with Q given unsymmetrically (single off-diagonal entry) — cuOpt symmetrizes internally (supported via NVIDIA#1361). Includes a linear row and uses MINIMIZE. Verified: Optimal, objective -2, x=y=-1. CI builds/runs both. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>

Reverts the over-conservative QP->LP eval switch. Git history confirms QP duals are real and recent: barrier extended for SOCP (NVIDIA#1290, 2026-05-30) and general convex quadratic constraints (NVIDIA#1361, 2026-06-02); the barrier solver is primal-dual and pdlp/solve.cu returns dual_solution + reduced_cost for the Barrier method. The api-python/api-c "dual values (LP only)" wording predates this (NVIDIA#1183, 2026-05-07) and is the stale part, not the formulation skill's "LP/QP" (NVIDIA#1393). The QP risk-return eval sweeps a *linear* return floor, whose dual is returned (cuOpt returns no dual for a quadratic constraint, so the quadratic stays the objective). Skill prose unchanged (already "LP/QP" + linear-constraint caveat). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: cafzal <cameron.afzal@gmail.com>

chris-maes requested review from a team as code owners June 1, 2026 22:44

chris-maes requested review from aliceb-nv, kaatish and ramakrishnap-nv June 1, 2026 22:44

chris-maes added non-breaking Introduces a non-breaking change improvement Improves an existing functionality labels Jun 1, 2026

chris-maes added this to the 26.06 milestone Jun 1, 2026

Merge remote-tracking branch 'cuopt-nvidia/release/26.06' into transf…
de8cd97
…orm_general_quadratic_constraints

coderabbitaiBot reviewed Jun 1, 2026
View reviewed changes

Style fixes
0e5869c

chris-maes added 2 commits June 1, 2026 17:11

Address coderabbit review
5d1a302

coderabbitaiBot reviewed Jun 2, 2026
View reviewed changes

chris-maes added 2 commits June 2, 2026 02:49

Merge remote-tracking branch 'cuopt-nvidia/release/26.06' into transf…
758a2fd
…orm_general_quadratic_constraints

Style fixes
3046bd8

Reject rotated cone with x^Tx <= 2*y*z with y free and z free. Only t…
1f1b2dc
…ranslate when y >= 0, z >= 0

yuwenchen95 requested changes Jun 2, 2026
View reviewed changes

yuwenchen95 approved these changes Jun 2, 2026
View reviewed changes

chris-maes added 4 commits June 2, 2026 10:06

Set dual variables and reduced costs to NaN on problems with quadrati…
1a31e51
…c constraints

Merge remote-tracking branch 'cuopt-nvidia/release/26.06' into transf…
f33da79
…orm_general_quadratic_constraints

Add unsymmetric quadratic matrix tests in C / Python
372fdfb

Bound propagation to catch head variable bounds on cones
e07e259

Don't use E rows for bound propagation
c7a9fda

chris-maes added 3 commits June 2, 2026 11:36

Right looking LDL^T arrowhead test
5697217

Merge remote-tracking branch 'cuopt-nvidia/release/26.06' into transf…
545fa1c
…orm_general_quadratic_constraints

Style fixes
c72d0cc

Only use derived bounds for verifying cone heads
c287553

ramakrishnap-nv approved these changes Jun 2, 2026
View reviewed changes

chris-maes removed request for aliceb-nv and kaatish June 2, 2026 19:24

Fix tests to allow for a nonzero rhs for quadratic constraints
e34b93c

rapids-botBot merged commit 400356a into NVIDIA:release/26.06Jun 2, 2026
98 checks passed

chris-maes deleted the transform_general_quadratic_constraints branch June 2, 2026 22:32

This was referenced Jun 3, 2026
[BUG] MAXIMIZE silently ignored (returns minimizer) when a quadratic constraint is present #1375
Closed
[BUG] Problem with only a quadratic constraint (no linear rows) returns nan #1376
Open

This was referenced Jun 8, 2026
Read epsilon-constraint duals as frontier slope / exchange rate in multi-objective skill #1406
Merged
Correct dual-value scope to LP/QP in numerical-optimization API skills #1408
Merged

		// Check for indefiniteness: a negative pivot means the matrix is not PSD
		if (pivot_val < 0) { return INDEFINITE_MATRIX_RETURN; }

Uh oh!

Conversation

chris-maes commented Jun 1, 2026

Uh oh!

copy-pr-botBot commented Jun 1, 2026

Uh oh!

coderabbitaiBot commented Jun 1, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitaiBot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitaiBotJun 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chris-maes commented Jun 2, 2026

Uh oh!

chris-maes commented Jun 2, 2026

Uh oh!

coderabbitaiBot left a comment

Choose a reason for hiding this comment

Uh oh!

chris-maes commented Jun 2, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chris-maes commented Jun 2, 2026

Uh oh!

chris-maes commented Jun 2, 2026

Uh oh!

ramakrishnap-nv commented Jun 2, 2026

Uh oh!

chris-maes commented Jun 2, 2026

Uh oh!

chris-maes commented Jun 2, 2026

Uh oh!

Uh oh!

Reviewers

coderabbitaiBot commented Jun 1, 2026•
edited
Loading