Extend the barrier solver to SOCP problems#1051
Conversation
Build the cone-local Jordan-product, scaling, and corrector kernels needed for the SOCP barrier path before augmented-system integration.
Wire the SOCP cone Hessian updates into the augmented solve path and fold in the follow-up cleanup that removes now-redundant kernel plumbing.
Tighten the augmented-system SOCP updates so the barrier path converges on the mixed LP/QP cone cases covered by the new regression tests.
Flatten the remaining SOCP barrier plumbing, remove superseded cone kernels, and harden presolve so linear columns stay ahead of the trailing cone block.
…hecks Add row-cone and presolve regression cases around the SOCP barrier path, clean up the PR-facing test wording, and align the cone layout validation with the shared infinity convention. Signed-off-by: Yan Zaretskiy <yzaretskiy@nvidia.com>
… note that we separate quadratical constraint from the constraint matrix A and RHS
Removed unused temporary file variable in test.
rg20
added
feature request
Signed-off-by: yuwenchen95 <yuwchen@nvidia.com>
| // x_j <= 1/a_ij * (rhs - upper_activity_i) | ||
| const i_t j = last_free_i; | ||
| const f_t a_ij = last_free_coeff_i; | ||
| if (a_ij == 0) { continue; } |
There was a problem hiding this comment.
Unnecessary as we have removed all 0 coefficients from the matrix.
| } | ||
| } | ||
| if (!(problem.lower[j] == -inf && problem.upper[j] == inf)) { | ||
| presolve_info.phase1_bounded_linear_indices.push_back(j); |
There was a problem hiding this comment.
We should not have a variable named phase_1_bounded_linear_indices
| } | ||
| } | ||
|
|
||
| // Barrier presolve phase 2: negate one-sided bounds (-inf < x <= u -> -u <= x < inf). |
There was a problem hiding this comment.
Remove "Barrier presolve phase 2" from comment.
|
|
||
| // The original problem may have nonzero lower bounds | ||
| // 0 != l_j <= x_j <= u_j | ||
| // Barrier presolve phase 3: shift nonzero lower bounds to zero (cone/stack columns not shifted). |
There was a problem hiding this comment.
Remove "Barrier phase 3". Put back the original comment that was deleted.
Fine to add something about "Cone variables should not be shifted"
| } | ||
|
|
||
| // Check for empty rows | ||
| // Barrier presolve phase 4: remove empty rows and empty linear columns. |
There was a problem hiding this comment.
Remove "Barrier presolve phase 4"
| if (settings.barrier_presolve && free_variables > 0 && problem.Q.n > 0) { | ||
| presolve_info.free_variable_indices.clear(); | ||
| for (i_t j = 0; j < problem.num_cols; j++) { | ||
| // Barrier presolve phase 5: free linear variables — 5a native (QP/SOCP) or 5b v-w split (LP). |
There was a problem hiding this comment.
Remove "Barrier presolve phase 5" comment. And "5a, 5b"
| for (i_t j = 0; j < linear_cols; j++) { | ||
| if (problem.lower[j] == -inf && problem.upper[j] == inf) { | ||
| presolve_info.free_variable_indices.push_back(j); | ||
| presolve_info.native_free_linear_indices.push_back(j); |
There was a problem hiding this comment.
Why are we changing from free_variable_indices to native_free_linear_indices? Let's keep the old name
| } | ||
| } else if (settings.barrier_presolve && free_variables > 0) { | ||
| settings.log.printf( | ||
| "Keeping %d native free linear variables for augmented-system barrier (QP/SOCP)\n", |
There was a problem hiding this comment.
"Keeping %d free variables"
| "Keeping %d native free linear variables for augmented-system barrier (QP/SOCP)\n", | ||
| native_free_count); | ||
| } else if (settings.barrier_presolve && !has_cones && free_variables > 0) { | ||
| // Phase 5b: x_j = v - w with v, w >= 0 (LP without cones; SOCP/QP use phase 5a). |
There was a problem hiding this comment.
Remove "Phase 5" and "phase 5a" from comment. You could delete this comment.
| // becomes | ||
| // sum_{k != j} c_k x_k + c_j v - c_j w | ||
|
|
||
| std::vector<i_t> pair_index(problem.num_cols, -1); |
There was a problem hiding this comment.
Why does any of this code need to change. This is for the LP case. Just make sure we aren't in an SOCP or QP and then use this code. I'd suggest reverting these changes.
| Q_j[row_starts[partner_row]] = partner_col; | ||
| Q_x[row_starts[partner_row]] = qij; | ||
| row_starts[partner_row]++; | ||
| } |
There was a problem hiding this comment.
Please revert all changes from line 1196/1135 to here.
| problem.A.i[q] = i; | ||
| problem.A.x[q] = -aij; | ||
| q++; | ||
| csc_matrix_t<i_t, f_t> expanded_A(problem.A.m, num_cols, nnz); |
There was a problem hiding this comment.
Can you do this expansion only in the SOCP case?
| } | ||
|
|
||
| if (!presolve_info.phase1_bounded_linear_indices.empty()) { | ||
| settings.log.printf("Post-solve: %d linear column(s) had phase-1 bound tightening (x unchanged)\n", |
There was a problem hiding this comment.
The user should not be expected to know the internal phases of presolve!
Remove this print and this block of code entirely.
| const i_t u = free_variable_pairs[k]; | ||
| const i_t v = free_variable_pairs[k + 1]; | ||
| input_x[u] -= input_x[v]; | ||
| remove_partner[v] = true; |
There was a problem hiding this comment.
Why is this code needed?
| settings.log.printf("Post-solve: Correcting duals for %d bounded free variables\n", | ||
| static_cast<i_t>(presolve_info.bounded_free_variables.size())); | ||
| const csc_matrix_t<i_t, f_t>& A = original_problem.A; | ||
| csr_matrix_t<i_t, f_t> A_row(A.m, A.n, A.nnz()); |
There was a problem hiding this comment.
Revert these changes. Converting to A_row is not needed
| if (w_j == 0.0) { continue; } | ||
| const f_t du = w_j / bfv.coefficient; | ||
| input_y[bfv.constraint] += du; | ||
| for (i_t j = 0; j < A.n; j++) { |
| } | ||
| settings.method = method_t::Barrier; | ||
| settings.presolver = presolver_t::None; | ||
| // Quadratic objective support is minimization-only. |
There was a problem hiding this comment.
Is this correct? I thought we supported maximizing with quadratic objectives.
| user_problem.Q_values = model.Q_values; | ||
|
|
||
| if (model.original_problem_ptr->has_quadratic_constraints()) { | ||
| detail::apply_soc_qcmatrix_conversion_for_simplex<i_t, f_t>( |
There was a problem hiding this comment.
for_simplex seems like a bad name here. As we aren't using the simplex solver. Maybe this was picked up because of the simplex namespace? Anyway, better to rename the function to avoid confusion.
| @@ -61,6 +65,29 @@ namespace cuopt::linear_programming::dual_simplex { | |||
| cuda::std::make_tuple(a, b), out, size, cuda::std::multiplies<>{}, stream.value()); | |||
| } | |||
|
|
|||
| // out[i] = is_native_free_linear[i] ? 0 : a[i] * b[i] | |||
| [[maybe_unused]] static void pairwise_multiply_skip_native_free_linear( | |||
There was a problem hiding this comment.
Nit: any reason not to template this on f_t to avoid having two identical versions?
| } | ||
| } else if (settings.check_Q) { | ||
| // TODO: Check to ensure that Q is positive semi-definite |
There was a problem hiding this comment.
Consider adding a warning that check_Q is ignored.
| RAFT_CUDA_TRY(cub::DeviceSelect::Flagged( | ||
| nullptr, | ||
| flag_buffer_size, | ||
| d_inv_diag_prime.data(), // Not the actual input but just to allcoate the memory |
| } else if (str == "UI") { | ||
| return UpperBoundIntegerVariable; | ||
| } else if (str == "SC") { | ||
| } else if (str == "SC" || str == "LC") { |
There was a problem hiding this comment.
This diff shouldn't be here, please revert this line.
There was a problem hiding this comment.
Specifically this is reverting eb3dac8#diff-1d4ef15136ad0cb10a2d09a2bada6bb3f324a1dc083b03d3a9df99db65367865
|
|
||
| namespace { | ||
|
|
||
| /** @brief ceil(log2(x)) for x >= 1; used for sparse-vs-dense CSR heuristic only. */ |
There was a problem hiding this comment.
This code is pretty confusing with three code paths(?) for processing CSR. Let's just have one code path until there's a clear reason to have more? LP also needs COO to CSR for the objective, right? let's share the code.
| csr_result.offsets); | ||
| ++quadratic_row_id; | ||
| const i_t nnz = static_cast<i_t>(block.entries.size()); | ||
| std::vector<i_t> qc_rows(static_cast<size_t>(nnz)); |
There was a problem hiding this comment.
I don't think we need a static_cast<size_t> everywhere, please remove.
| row_id, | ||
| row_names[row_id], | ||
| static_cast<char>(row_types[row_id]), | ||
| std::span<const f_t>(A_values[row_id].data(), A_values[row_id].size()), |
There was a problem hiding this comment.
I think you can pass A_values (etc) directly, explicit span construction shouldn't be needed.
| "${dejavu_SOURCE_DIR}" | ||
| ) | ||
| # Third-party headers: SYSTEM matches libcuopt and suppresses NVCC sign-conversion noise. | ||
| target_include_directories(cuopttestutils SYSTEM PRIVATE "${dejavu_SOURCE_DIR}") |
There was a problem hiding this comment.
Unrelated diff? please revert
| Apache 2.0 <br> | ||
| ## Use Case: <br> | ||
| Developers and engineers use this skill to capture generalizable learnings from agent interactions and propose updates to existing skills, ensuring continuous improvement of agent guidance without blocking the user's primary task. <br> | ||
| Developers and engineers use this skill to continuously improve agent skill files by capturing generalizable learnings from problem-solving interactions and proposing structured updates. <br> |
| ConfigureTest(DUAL_SIMPLEX_TEST | ||
| ${CMAKE_CURRENT_SOURCE_DIR}/unit_tests/solve.cpp | ||
| ${CMAKE_CURRENT_SOURCE_DIR}/unit_tests/solve_barrier.cu | ||
| LABELS numopt) |
There was a problem hiding this comment.
Do not delete the test label
rg20
left a comment
rg20
left a comment
There was a problem hiding this comment.
I reviewed only a subset of files. I will continue to review.
| std::vector<i_t> linear_indices{}; | ||
| f_t rhs_value{f_t(0)}; | ||
| /** Q in COO: parallel arrays, same length. */ | ||
| std::vector<i_t> quadratic_row_indices{}; |
There was a problem hiding this comment.
| std::vector<i_t> quadratic_row_indices{}; | |
| std::vector<i_t> rows{}; |
| lp_problem_t<i_t, f_t>& scaled, | ||
| std::vector<f_t>& column_scaling) | ||
| std::vector<f_t>& col_scale, | ||
| std::vector<f_t>& row_scale) |
There was a problem hiding this comment.
Looks like row_scaling is not used to scale matrix A?
| row_scale.assign(m, 1.0); | ||
|
|
||
| // ========================================================================= | ||
| // Ruiz equilibration for SOCP (and QP) problems |
There was a problem hiding this comment.
Can we remove this scaling?
We should do this cleanly on the KKT system.
| } | ||
|
|
||
| template <typename i_t, typename f_t> | ||
| bool validate_barrier_cone_layout(const lp_problem_t<i_t, f_t>& problem, |
There was a problem hiding this comment.
This function does not belong here. Move this to src/barrier
|
This PR is moved to #1290 . Please make all comments and changes on that PR. |
10 participants
This is a draft PR that 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, both of which can be specified in the CBF format. It uses Nesterov-Todd scaling to make the conic diagonal blocks symmetric.