Extend the barrier solver for Second-Order Cone Programming (SOCP)#1290
Merged
rapids-bot[bot] merged 62 commits intoMay 30, 2026
Merged
Conversation
This PR extends the 2x2 augmented system formulation of the barrier solver to solve SOCP problems. It supports both explicit cone variables as well as the cone rows in the Ax=b constraint. It uses Nesterov-Todd scaling to make the conic diagonal blocks symmetric. Co-authored-by: Yan Zaretskiy <yan@fastmail.com> Co-authored-by: Yan Zaretskiy <yzaretskiy@nvidia.com> Co-authored-by: Yuwen Chen <yuwchen@nvidia.com>
chris-maes
added
non-breaking
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR implements comprehensive second-order cone programming (SOCP) and quadratic-constraint support across the CUOPT stack. Changes migrate quadratic matrices from CSR to COO format, extend presolve to handle cone variables separately, add two-dimensional scaling with Ruiz equilibration, integrate barrier solver validation, and provide complete Python bindings for quadratic constraint modeling and I/O. ChangesSOCP and Quadratic Constraint Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cpp/src/barrier/dense_vector.hpp (1)
11-14:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
<limits>to keepcpp/src/barrier/dense_vector.hppself-contained.
dense_vector.hppusesstd::numeric_limits(line 195) but does not include<limits>, so it relies on transitive includes (IWYU fragile).🔧 Proposed fix
`#include` <cmath> `#include` <cstdio> +#include <limits> `#include` <vector>🤖 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/dense_vector.hpp` around lines 11 - 14, The header dense_vector.hpp is using std::numeric_limits (referenced around the usage at line ~195) but doesn't include <limits>, causing a fragile IWYU dependency; fix by adding an `#include` <limits> at the top of cpp/src/barrier/dense_vector.hpp so the header is self-contained and std::numeric_limits is available to functions/classes in that file (e.g., where numeric_limits is used).cpp/src/dual_simplex/simplex_solver_settings.hpp (1)
49-83:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep
barrier_presolveopt-in at the barrier call site.Changing the shared default to
trueaffects more than barrier solves.run_dual_simplex()incpp/src/pdlp/solve.custill default-constructssimplex_solver_settings_tand does not override this flag before calling into presolve, so dual-simplex now inherits barrier-specific presolve behavior too. If the intent is only to preserve the barrier path after removing the localbarrier_settingscopy, setbarrier_presolve = truethere instead of flipping the global default.Also applies to: 166-166
🤖 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/simplex_solver_settings.hpp` around lines 49 - 83, The constructor for simplex_solver_settings_t currently sets barrier_presolve = true, which unintentionally enables barrier-specific presolve for dual-simplex callers that default-construct simplex_solver_settings_t (e.g., run_dual_simplex in pdlp/solve.cu); revert the global default by setting barrier_presolve back to false in simplex_solver_settings_t, and instead explicitly enable barrier_presolve = true only where the barrier path is invoked (restore the previous behavior by setting barrier_presolve on the barrier_settings copy or before calling the barrier routine in the barrier-specific call site such as the code that previously owned barrier_settings).
🧹 Nitpick comments (2)
cpp/include/cuopt/linear_programming/pdlp/solver_settings.hpp (1)
283-285: ⚡ Quick winDocument the new barrier tolerance semantics inline.
These three new numerical controls should include brief field comments describing which residual each one governs and intended value range/behavior.
As per coding guidelines "Flag missing documentation for numerical tolerances and algorithm parameters."
🤖 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/include/cuopt/linear_programming/pdlp/solver_settings.hpp` around lines 283 - 285, Add inline field comments for barrier_relative_feasibility_tolerance, barrier_relative_optimality_tolerance, and barrier_relative_complementarity_tolerance that state which residual each parameter controls (feasibility residual, optimality/residual gradient, and complementarity gap respectively), include suggested value ranges (e.g. typical defaults ~1e-8 and acceptable range like 1e-12..1e-4) and note behavior (smaller values tighten termination, larger values relax it); place these brief comments immediately above or to the right of the three fields in solver_settings.hpp so the semantics are clear to readers and follow the project's documentation guideline for numerical tolerances.cpp/src/dual_simplex/scaling.cpp (1)
57-173: ⚡ Quick winDocument the new Ruiz heuristics as solver parameters.
100.0,10,0.1, and the1e20bound sentinel now directly affect scaling behavior, but they are still anonymous literals inside the solver core. Please at least lift them into named constants with rationale, or preferably settings, so regressions are traceable and tuning is deliberate. As per coding guidelines, "Flag missing documentation for numerical tolerances and algorithm parameters".🤖 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/scaling.cpp` around lines 57 - 173, The Ruiz equilibration uses hard-coded literals (100.0 row norm ratio, 10 max iterations, 0.1 deviation tolerance, and 1e20 bound sentinel) — lift them into named parameters (preferably on the existing settings object) and replace the anonymous literals in functions/blocks around Ruiz equilibration (check row_norm_ratio comparison, max_ruiz_iterations, the max_deviation break threshold, and the bound checks using 1e20), then update the log message ("Ruiz equilibration: coefficient range ...") to include the effective settings values; name suggestions: settings.ruiz_row_ratio_threshold, settings.ruiz_max_iterations, settings.ruiz_deviation_tolerance, settings.bound_sentinel (or constants with those names if not exposed) so future tuning and documentation can reference them.
🤖 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 939-940: The code caches linear_cols = linear_var_count(problem)
before presolve but then calls remove_empty_cols which can shrink the linear
block, so later uses of linear_cols (e.g., in the logic around cone_var_start,
second_order_cone_dims and accesses to
problem.lower/problem.upper/problem.objective) can be out-of-date and index past
vectors; recompute linear_cols immediately after any column-removal/presolve
passes (specifically after remove_empty_cols) and use the refreshed linear_cols
when deciding free-variable handling and when computing cone_var_start/iterating
second_order_cone_dims to ensure all indexing into
problem.lower/problem.upper/problem.objective is within bounds.
In `@cpp/src/dual_simplex/scaling.cpp`:
- Around line 36-63: The Ruiz scaling branch for SOCP/QP runs before honoring
settings.scale_columns, so callers cannot disable column scaling for
quadratic/conic models; update the logic in scaling.cpp to check
settings.scale_columns before entering the Ruiz equilibration branch (or add
settings.scale_columns to the condition that gates Ruiz), ensuring the existing
guard (settings.scale_columns) is evaluated prior to the block that constructs
Arow_check and computes row_norm_ratio (refer to the Ruiz equilibration code and
the settings.scale_columns flag), and apply the same fix for the similar branch
around the later block referenced in the comment (the section around the lines
noted as 177-180).
In `@cpp/src/dual_simplex/solve.cpp`:
- Around line 64-97: The function validate_barrier_cone_layout uses
problem.cone_var_start as an index into problem.lower/problem.upper without
validating it; add an explicit check near the top of
validate_barrier_cone_layout (before computing cone_end or iterating bounds)
that cone_var_start is within [0, problem.num_cols] (and that cone_var_start <=
problem.num_cols) and if not call settings.log.printf with a clear error (e.g.
"Error: invalid cone_var_start") and return false so out-of-range indexing on
problem.lower/problem.upper is prevented.
In `@cpp/src/io/mps_data_model.cpp`:
- Around line 163-187: Check and reject out-of-range or negative index values
immediately after the existing length/null checks: iterate the span
linear_indices (for linear_values.size()) and ensure each index is within [0,
num_variables) (replace num_variables with the actual variable/column count
symbol used in this file), calling mps_parser_expects on the first offending
value; similarly iterate quadratic_row_indices and quadratic_col_indices for
q_nnz entries and validate each is in [0, num_variables) (or appropriate row/col
bounds if different), rejecting any negative or too-large index with a clear
ValidationError message that includes the offending index and its position.
In `@cpp/src/io/mps_writer.cpp`:
- Around line 505-517: The loop uses nnz from qc.quadratic_values.size() but
indexes qc.quadratic_row_indices and qc.quadratic_col_indices without
validation; add input validation before the loop (e.g., in the function that
writes QCMATRIX) to ensure qc.quadratic_row_indices.size() and
qc.quadratic_col_indices.size() are at least nnz and handle the mismatch by
returning an error/throwing or logging and aborting export. Specifically, check
sizes of quadratic_values, quadratic_row_indices, and quadratic_col_indices
(symbols: nnz, qc.quadratic_values, qc.quadratic_row_indices,
qc.quadratic_col_indices) and fail fast if any are shorter than nnz so the
for-loop that reads indices cannot read out-of-bounds.
In `@python/cuopt/cuopt/linear_programming/data_model/data_model.py`:
- Around line 293-375: The new public DataModel methods
get_quadratic_constraints, clear_quadratic_constraints, and
add_quadratic_constraint lack type hints and complete docstring sections; update
their signatures to include precise type annotations (e.g., return types:
get_quadratic_constraints -> List[Dict[str, Any]] or appropriate TypedDict,
clear_quadratic_constraints -> None, add_quadratic_constraint parameter types
like constraint_row_index: int, constraint_row_name: str = "", linear_values:
Optional[Sequence[float]] = None, linear_indices: Optional[Sequence[int]] =
None, rhs_value: float = 0.0, quadratic_values: Optional[Sequence[float]] =
None, quadratic_row_indices: Optional[Sequence[int]] = None,
quadratic_col_indices: Optional[Sequence[int]] = None, sense: Union[str, Enum] =
"L" and return type -> None) and expand each docstring to include Parameters,
Returns, and Raises sections consistent with project style (document possible
ValueError raises in add_quadratic_constraint and any exceptions raised by
`@catch_cuopt_exception`). Ensure imports for typing (List, Dict, Any, Optional,
Sequence, Union) are added if missing and preserve existing logic in the
function bodies.
In `@python/cuopt/cuopt/linear_programming/problem.py`:
- Around line 1983-1988: The public method getQuadraticConstraints lacks a
return type hint and a documented Returns section; add a type annotation such as
-> List[Constraint] (or -> List["Constraint"] if Constraint is not yet defined)
and import List from typing at the top of the module, then expand the docstring
to include a Returns section describing that it returns a list of constraint
objects representing quadratic (QCMATRIX) constraints (e.g., "Returns:
List[Constraint]: All quadratic constraints in the problem."). Keep the
implementation unchanged: it should still return [c for c in self.constrs if
c.is_quadratic].
---
Outside diff comments:
In `@cpp/src/barrier/dense_vector.hpp`:
- Around line 11-14: The header dense_vector.hpp is using std::numeric_limits
(referenced around the usage at line ~195) but doesn't include <limits>, causing
a fragile IWYU dependency; fix by adding an `#include` <limits> at the top of
cpp/src/barrier/dense_vector.hpp so the header is self-contained and
std::numeric_limits is available to functions/classes in that file (e.g., where
numeric_limits is used).
In `@cpp/src/dual_simplex/simplex_solver_settings.hpp`:
- Around line 49-83: The constructor for simplex_solver_settings_t currently
sets barrier_presolve = true, which unintentionally enables barrier-specific
presolve for dual-simplex callers that default-construct
simplex_solver_settings_t (e.g., run_dual_simplex in pdlp/solve.cu); revert the
global default by setting barrier_presolve back to false in
simplex_solver_settings_t, and instead explicitly enable barrier_presolve = true
only where the barrier path is invoked (restore the previous behavior by setting
barrier_presolve on the barrier_settings copy or before calling the barrier
routine in the barrier-specific call site such as the code that previously owned
barrier_settings).
---
Nitpick comments:
In `@cpp/include/cuopt/linear_programming/pdlp/solver_settings.hpp`:
- Around line 283-285: Add inline field comments for
barrier_relative_feasibility_tolerance, barrier_relative_optimality_tolerance,
and barrier_relative_complementarity_tolerance that state which residual each
parameter controls (feasibility residual, optimality/residual gradient, and
complementarity gap respectively), include suggested value ranges (e.g. typical
defaults ~1e-8 and acceptable range like 1e-12..1e-4) and note behavior (smaller
values tighten termination, larger values relax it); place these brief comments
immediately above or to the right of the three fields in solver_settings.hpp so
the semantics are clear to readers and follow the project's documentation
guideline for numerical tolerances.
In `@cpp/src/dual_simplex/scaling.cpp`:
- Around line 57-173: The Ruiz equilibration uses hard-coded literals (100.0 row
norm ratio, 10 max iterations, 0.1 deviation tolerance, and 1e20 bound sentinel)
— lift them into named parameters (preferably on the existing settings object)
and replace the anonymous literals in functions/blocks around Ruiz equilibration
(check row_norm_ratio comparison, max_ruiz_iterations, the max_deviation break
threshold, and the bound checks using 1e20), then update the log message ("Ruiz
equilibration: coefficient range ...") to include the effective settings values;
name suggestions: settings.ruiz_row_ratio_threshold,
settings.ruiz_max_iterations, settings.ruiz_deviation_tolerance,
settings.bound_sentinel (or constants with those names if not exposed) so future
tuning and documentation can reference them.
🪄 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: adbdcc6b-fcfc-43b0-b6b8-9cb53032b5d9
📒 Files selected for processing (37)
cpp/include/cuopt/linear_programming/constants.hcpp/include/cuopt/linear_programming/io/data_model_view.hppcpp/include/cuopt/linear_programming/io/mps_data_model.hppcpp/include/cuopt/linear_programming/optimization_problem_interface.hppcpp/include/cuopt/linear_programming/pdlp/solver_settings.hppcpp/src/barrier/barrier.cucpp/src/barrier/barrier.hppcpp/src/barrier/dense_vector.hppcpp/src/dual_simplex/presolve.cppcpp/src/dual_simplex/presolve.hppcpp/src/dual_simplex/scaling.cppcpp/src/dual_simplex/scaling.hppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/solve.cppcpp/src/dual_simplex/user_problem.hppcpp/src/dual_simplex/vector_math.cuhcpp/src/io/lp_parser.cppcpp/src/io/mps_data_model.cppcpp/src/io/mps_parser.cppcpp/src/io/mps_writer.cppcpp/src/math_optimization/solver_settings.cucpp/src/pdlp/solve.cucpp/src/pdlp/termination_strategy/infeasibility_information.cucpp/src/pdlp/translate.hppcpp/tests/dual_simplex/CMakeLists.txtcpp/tests/dual_simplex/unit_tests/solve_barrier.cucpp/tests/linear_programming/parser_test.cppcpp/tests/qp/unit_tests/no_constraints.cucpp/tests/qp/unit_tests/two_variable_test.cupython/cuopt/cuopt/linear_programming/data_model/data_model.pxdpython/cuopt/cuopt/linear_programming/data_model/data_model.pypython/cuopt/cuopt/linear_programming/data_model/data_model_wrapper.pxdpython/cuopt/cuopt/linear_programming/data_model/data_model_wrapper.pyxpython/cuopt/cuopt/linear_programming/io/parser.pxdpython/cuopt/cuopt/linear_programming/io/parser_wrapper.pyxpython/cuopt/cuopt/linear_programming/problem.pypython/cuopt/cuopt/tests/linear_programming/test_python_API.py
💤 Files with no reviewable changes (1)
- cpp/src/pdlp/termination_strategy/infeasibility_information.cu
Comment on lines
+64
to
+97
| template <typename i_t, typename f_t> | ||
| bool validate_barrier_cone_layout(const lp_problem_t<i_t, f_t>& problem, | ||
| const simplex_solver_settings_t<i_t, f_t>& settings) | ||
| { | ||
| if (problem.second_order_cone_dims.empty()) { return true; } | ||
|
|
||
| i_t cone_end = problem.cone_var_start; | ||
| for (auto q_k : problem.second_order_cone_dims) { | ||
| if (q_k <= 1) { | ||
| settings.log.printf( | ||
| "Error: second-order cone dimensions must be at least 2; use linear variables instead of " | ||
| "Q^1\n"); | ||
| return false; | ||
| } | ||
| cone_end += q_k; | ||
| } | ||
|
|
||
| if (cone_end != problem.num_cols) { | ||
| settings.log.printf("Error: conic variables must form a trailing block [linear | cone]\n"); | ||
| return false; | ||
| } | ||
|
|
||
| for (i_t j = problem.cone_var_start; j < cone_end; ++j) { | ||
| if (problem.lower[j] != 0.0 && problem.lower[j] > -inf) { | ||
| settings.log.printf("Error: explicit lower bound on conic variable %d is not supported\n", j); | ||
| return false; | ||
| } | ||
| if (problem.upper[j] < inf) { | ||
| settings.log.printf("Error: explicit upper bound on conic variable %d is not supported\n", j); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| return true; |
There was a problem hiding this comment.
Validate cone_var_start before using it as an index.
This helper checks cone sizes and the trailing-block invariant, but it never verifies that cone_var_start itself is in [0, num_cols] before using it to drive the bounds loop. A malformed converted problem can still walk lower/upper out of range here instead of failing cleanly.
Suggested fix
bool validate_barrier_cone_layout(const lp_problem_t<i_t, f_t>& problem,
const simplex_solver_settings_t<i_t, f_t>& settings)
{
if (problem.second_order_cone_dims.empty()) { return true; }
+
+ if (problem.cone_var_start < 0 || problem.cone_var_start > problem.num_cols) {
+ settings.log.printf("Error: cone_var_start must be in [0, num_cols]\n");
+ return false;
+ }
i_t cone_end = problem.cone_var_start;🤖 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/solve.cpp` around lines 64 - 97, The function
validate_barrier_cone_layout uses problem.cone_var_start as an index into
problem.lower/problem.upper without validating it; add an explicit check near
the top of validate_barrier_cone_layout (before computing cone_end or iterating
bounds) that cone_var_start is within [0, problem.num_cols] (and that
cone_var_start <= problem.num_cols) and if not call settings.log.printf with a
clear error (e.g. "Error: invalid cone_var_start") and return false so
out-of-range indexing on problem.lower/problem.upper is prevented.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cpp/src/pdlp/solve.cu (1)
1796-1827:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMove the quadratic-objective sense check before the early
solve_qp()return.
solve_lp()returns tosolve_qp()before this new minimization-only guard runs, so a maximization QP still goes down the barrier path instead of being rejected. Put the sense check before the early return, or mirror it insidesolve_qp().🛠️ Minimal fix
- if (op_problem.has_quadratic_objective()) { - return solve_qp(op_problem, settings_const, problem_checking); - } + if (op_problem.has_quadratic_objective() && op_problem.get_sense()) { + CUOPT_LOG_ERROR("Quadratic problems must be minimized"); + return optimization_problem_solution_t<i_t, f_t>( + pdlp_termination_status_t::NumericalError, op_problem.get_handle_ptr()->get_stream()); + } + if (op_problem.has_quadratic_objective()) { + return solve_qp(op_problem, settings_const, problem_checking); + }As per coding guidelines, flag missing input validation at library and server boundaries.
🤖 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/pdlp/solve.cu` around lines 1796 - 1827, The minimization-only check for quadratic objectives is executed after the early return to solve_qp(), so maximization QPs can bypass the guard; move the sense check for quadratic objectives (the op_problem.has_quadratic_objective() && op_problem.get_sense() branch that logs "Quadratic problems must be minimized") so it runs before the conditional that returns solve_qp(op_problem, settings_const, problem_checking), or alternatively add the same sense validation inside solve_qp(); update the relevant logic in solve_lp()/solve_qp() to ensure any quadratic-objective problem is rejected with CUOPT_LOG_ERROR and returns pdlp_termination_status_t::NumericalError for non-minimization cases.cpp/src/dual_simplex/presolve.cpp (1)
289-393:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftRemap
Qwith the post-slack column order.In the cone-aware
<=conversion, slacks are inserted before the cone block, butproblem.Qis still populated later fromuser_problem.Q_*. That means the remap block here never runs, and the later load keeps quadratic row/column indices in the pre-insertion layout. Any QP/SOCP instance withLrows will attach quadratic terms to the wrong columns.Either build
problem.Qbeforeconvert_less_than_to_equal()so this remap executes, or rebuildQfromold_to_newafter the slack insertion finalizes the new column order.As per coding guidelines, this PR should prioritize correctness in problem transformations and avoid index-mapping mistakes across presolve/scaling/uncrush.
Also applies to: 915-933
🤖 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 289 - 393, The remapping of quadratic matrix problem.Q is happening before the final column order (post-slack insertion) is guaranteed, so Q indices remain in the pre-insertion layout; fix by rebuilding/remapping problem.Q after slacks are inserted using the old_to_new mapping (use old_Q and old_to_new to populate problem.Q.j and problem.Q.x) or by ensuring problem.Q is constructed before calling convert_less_than_to_equal(); specifically locate the slack-insertion block that produces expanded_A, new_slacks, old_to_new and then, after finalizing num_cols/new_cone_start and expanded_A assignment, reconstruct problem.Q from old_Q using old_to_new (update problem.Q.row_start, problem.Q.j, problem.Q.x, problem.Q.m/n/nz_max) so quadratic row/column indices match the post-slack column order.
🧹 Nitpick comments (1)
cpp/tests/dual_simplex/unit_tests/solve_barrier.cu (1)
997-1030: 💤 Low valueConsider removing debug printf statement.
Line 1024 contains a
printfdebug statement that may have been left from development. Consider removing it for cleaner test output.- printf("x %e y %e z %e\n", h_x[0], h_y[0], h_z[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/dual_simplex/unit_tests/solve_barrier.cu` around lines 997 - 1030, The test barrier::min_x_squared_free_variable_dual_correction contains a leftover debug printf("x %e y %e z %e\n", h_x[0], h_y[0], h_z[0]) — remove this printf call from the TEST(min_x_squared_free_variable_dual_correction) body so the unit test output stays clean; keep the EXPECT_NEAR assertions and other code unchanged.
🤖 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.
Outside diff comments:
In `@cpp/src/dual_simplex/presolve.cpp`:
- Around line 289-393: The remapping of quadratic matrix problem.Q is happening
before the final column order (post-slack insertion) is guaranteed, so Q indices
remain in the pre-insertion layout; fix by rebuilding/remapping problem.Q after
slacks are inserted using the old_to_new mapping (use old_Q and old_to_new to
populate problem.Q.j and problem.Q.x) or by ensuring problem.Q is constructed
before calling convert_less_than_to_equal(); specifically locate the
slack-insertion block that produces expanded_A, new_slacks, old_to_new and then,
after finalizing num_cols/new_cone_start and expanded_A assignment, reconstruct
problem.Q from old_Q using old_to_new (update problem.Q.row_start, problem.Q.j,
problem.Q.x, problem.Q.m/n/nz_max) so quadratic row/column indices match the
post-slack column order.
In `@cpp/src/pdlp/solve.cu`:
- Around line 1796-1827: The minimization-only check for quadratic objectives is
executed after the early return to solve_qp(), so maximization QPs can bypass
the guard; move the sense check for quadratic objectives (the
op_problem.has_quadratic_objective() && op_problem.get_sense() branch that logs
"Quadratic problems must be minimized") so it runs before the conditional that
returns solve_qp(op_problem, settings_const, problem_checking), or alternatively
add the same sense validation inside solve_qp(); update the relevant logic in
solve_lp()/solve_qp() to ensure any quadratic-objective problem is rejected with
CUOPT_LOG_ERROR and returns pdlp_termination_status_t::NumericalError for
non-minimization cases.
---
Nitpick comments:
In `@cpp/tests/dual_simplex/unit_tests/solve_barrier.cu`:
- Around line 997-1030: The test
barrier::min_x_squared_free_variable_dual_correction contains a leftover debug
printf("x %e y %e z %e\n", h_x[0], h_y[0], h_z[0]) — remove this printf call
from the TEST(min_x_squared_free_variable_dual_correction) body so the unit test
output stays clean; keep the EXPECT_NEAR assertions and other code unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d5a99db0-4541-4bb7-afd6-bba771f2a3f8
📒 Files selected for processing (10)
cpp/src/barrier/barrier.cucpp/src/dual_simplex/presolve.cppcpp/src/dual_simplex/presolve.hppcpp/src/dual_simplex/scaling.cppcpp/src/dual_simplex/solve.cppcpp/src/pdlp/solve.cucpp/tests/dual_simplex/unit_tests/solve_barrier.cupython/cuopt/cuopt/linear_programming/data_model/data_model.pypython/cuopt/cuopt/linear_programming/data_model/data_model_wrapper.pxdpython/cuopt/cuopt/linear_programming/problem.py
💤 Files with no reviewable changes (1)
- python/cuopt/cuopt/linear_programming/data_model/data_model.py
…aints_to_second_order_cones
chris-maes
commented
May 23, 2026
Contributor
|
/ok to test 3fc42b7 |
Contributor
|
/ok to test 988ece4 |
chris-maes
commented
May 29, 2026
chris-maes
commented
May 29, 2026
chris-maes
commented
May 29, 2026
chris-maes
changed the title
chris-maes
changed the title
Contributor
|
/ok to test b264a17 |
yuwenchen95
force-pushed
the
socp-barrier-solver-2
branch
from
b264a17 to
1362c5b
Compare
Contributor
|
/ok to test 1362c5b |
rg20
approved these changes
May 29, 2026
Contributor
rg20
left a comment
rg20
left a comment
There was a problem hiding this comment.
Most of my comments relate to cosmetic changes.
|
|
||
| std::vector<size_t> perm(q_nnz); | ||
| std::iota(perm.begin(), perm.end(), size_t{0}); | ||
| std::stable_sort(perm.begin(), perm.end(), [&](size_t a, size_t b) { |
Contributor
There was a problem hiding this comment.
Why do you need stable sort? Are there duplicates? If so they have to be handled as well.
Contributor
Author
There was a problem hiding this comment.
@yuwenchen95 can you answer this when you have a chance?
Contributor
There was a problem hiding this comment.
I think sort is enough since we would throw an error in translation step if duplicate items appear. Corrected it in the new commit.
…nted system. Revert new_mu. Clean up bound split loop
Contributor
Author
|
/ok to test 475aa20 |
Contributor
Author
|
/ok to test aaefe54 |
Contributor
Author
|
/ok to test f1711a7 |
…ions back from the expanded SOC layout, less duplicated direct-free-variable logic in the barrier kernel host code, removal of an redundant presolve override, and a minor parser sort simplification. No intended behavior change except callers of solve_linear_program_with_barrier no longer get presolve forced on unless they set it.
Contributor
|
/ok to test 2736181 |
Contributor
Author
|
/merge |
rapids-bot
Bot
merged commit be1b208
into
NVIDIA:release/26.06
185 of 192 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extend the barrier solver to handle second-order cone programming (SOCP) specified via quadratic constraints (x^T Q x + a^T x <= rhs).
Algorithmic changes:
Public API:
Presolve:
Code quality and refactoring:
Tests: