Repository navigation
Conversation
Signed-off-by: yuwenchen95 <[email protected]>
This PR fixes build after the latest RMM merge broke our pipeline (rapidsai/rmm@6646d15). device_scalar no longer accepts a r-value constructor. Replaced with common constants as inline constexpr that are passed instead of r-value constants. <!-- Add brief description here --> <!-- Add closes #ISSUE_NUMBER here, this would close the issue once PR is merged, if there is no issue, please feel free to remove this section --> - [ ] I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/cuopt/blob/HEAD/CONTRIBUTING.md). - 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
The cache-reuse rebind left one destructor check as settings_. instead of settings_->, which only compiles on CU13 wheels. Signed-off-by: root <[email protected]>
Keep cache-reuse symbolic_done_ and main's explicit CUstream initialization. Signed-off-by: Ishika Roy <[email protected]>
Signed-off-by: yuwenchen95 <[email protected]>
…augmented path, and the never-read device_A_x_values snapshot in iteration_data_t; no numerical change. Signed-off-by: yuwenchen95 <[email protected]>
Crush a new user-space constraint RHS into the cached barrier workspace so a sequence re-solve can skip convert/presolve/scaling, mirroring update_linear_objective. - crush_user_rhs in barrier_transform.hpp negates 'G' rows, checks the rows presolve dropped as empty, gathers remaining_constraints and divides by row_scales. rhs_shift and rhs_update_supported are recorded on the first solve; range rows and folding are refused. - Empty rows dropped at t=0 are tested against the solve's primal_tol rather than exact zero, and an infeasible one short-circuits the next Solve to INFEASIBLE without running IPM. - The single c_dirty flag becomes dirty()/mark_clean() over separate c/b flags so further update APIs can reuse the same gate. Signed-off-by: Ishika Roy <[email protected]>
Recovered from 7d61502; it was deleted by the 698afbe log cleanup. Updated for the update_rhs naming and the deferred setter cache-invalidation gap. Signed-off-by: Ishika Roy <[email protected]>
…e_apis Signed-off-by: Ishika Roy <[email protected]> # Conflicts: # cpp/src/barrier/barrier.cu # cpp/src/barrier/device_sparse_matrix.cuh
new_slacks.empty() was far too strict: convert_less_than_to_equal adds a slack for every inequality row, so any model with an inequality was refused. Only convert_range_rows destroys the RHS (it zeroes rhs[i] and moves the bounds onto the slack); artificials leave rhs alone and convert_greater_to_less negates it, which the crush already mirrors. Gate on num_range_rows instead. Verified with a QP over a G row: two successive update_rhs re-solves take the reuse path, skip presolve / reordering / symbolic factorization, and match a fresh full solve. Signed-off-by: Ishika Roy <[email protected]>
The repo had no sequence_solve coverage at all. These compare every cached re-solve against a fresh full solve of the same model, so no assertion depends on a hand-derived optimum, and each asserts the reuse log line so a test cannot pass while the gate quietly rejects the model and falls back to a full solve. Models force the crush paths a one-row QP leaves as no-ops: mixed E/L/G senses, row norms seven orders of magnitude apart (non-unit row_scales), nonzero variable lower bounds (rhs_shift of -7; dropping it moves the optimum 115%), and an empty row presolve drops, covering both the feasible case and the short-circuit to PrimalInfeasible with no IPM and a surviving cache. Checked by mutation: removing barrier_presolve_bound_free_variables=0 fails 5 of the 6, all reporting the fallback. Signed-off-by: Ishika Roy <[email protected]>
Signed-off-by: Ishika Roy <[email protected]>
…gmented sort, reuse it for a new on-device transpose so the augmented path drops the host A^T build and upload, and cover both with a unit test. Signed-off-by: yuwenchen95 <[email protected]>
Signed-off-by: yuwenchen95 <[email protected]> # Conflicts: # cpp/src/barrier/device_sparse_matrix.cuh
| impl_->rhs_dirty = true; | ||
| } | ||
|
|
||
| template void barrier_cache_t::store_transform<int, double>( |
There was a problem hiding this comment.
We usually wrap template initialization in an ifdef. Please take a look at some of the other files in the barrier directory for an example
| std::vector<f_t> const& expanded_objective) | ||
| { | ||
| std::vector<f_t> problem_objective(static_cast<std::size_t>(problem_num_cols(xf))); | ||
| for (i_t j = 0; j < static_cast<i_t>(problem_objective.size()); ++j) { |
There was a problem hiding this comment.
Nit: avoid all the static_casts in this loop
There was a problem hiding this comment.
The cast keeps the loop index an i_t while vector::size() is size_t
| std::vector<value_t> const& problem_objective) | ||
| { | ||
| std::vector<value_t> expanded(static_cast<std::size_t>(xf.user_num_cols), value_t(0)); | ||
| for (i_t j = 0; j < static_cast<i_t>(problem_objective.size()); ++j) { |
There was a problem hiding this comment.
Nit: avoid all the static casts in this loop
There was a problem hiding this comment.
The cast keeps the loop index an i_t while vector::size() is size_t
| original[static_cast<std::size_t>(i)] = | ||
| xf.row_sense[static_cast<std::size_t>(i)] == 'G' ? -b[i] : b[i]; | ||
| const std::size_t user_num_rows = xf.user_num_rows; | ||
| for (std::size_t i = 0; i < user_num_rows; ++i) { |
There was a problem hiding this comment.
const i_t user_num_rows = xf.user_num_rows;
for (i_t i = 0; i < user_num_rows; ++i)
| op_problem.has_quadratic_objective(), | ||
| false); | ||
| // Must stay in lockstep with the gate in solve_linear_program_with_barrier: this path swaps | ||
| // in the slim user_problem_from_transform, so disagreement runs presolve on a fabricated |
There was a problem hiding this comment.
What is a fabricated problem?
There was a problem hiding this comment.
user_problem_from_transform builds a stand-in problem, not the converted model. The wording is changed now
|
/ok to test 2e1a10e |
CI Test Summary✅ All 9 test job(s) passed. (4 skipped) |
|
/ok to test 2f022ba |
yuwenchen95
left a comment
There was a problem hiding this comment.
Overall is good to me, with some minor comments. Thanks!
| bool barrier; // true to use barrier method, false to use dual simplex method | ||
| // Equality substitution of zero-cost free variables. A barrier cache cannot refresh the | ||
| // stored pivot RHS, so a solve that fills the cache turns this off. | ||
| bool barrier_eliminate_free_variables = true; |
There was a problem hiding this comment.
nit: every other field here is initialized in the constructor's init list; could this follow the same pattern (barrier_eliminate_free_variables(true) right after barrier_presolve(false)) with a one-line trailing comment like its neighbors?
| { | ||
| lp_status_t status = lp_status_t::UNSET; | ||
| simplex_solver_settings_t<i_t, f_t> barrier_settings = settings; | ||
| // The cached presolve records cannot be replayed after an RHS update, so the fill solve |
There was a problem hiding this comment.
It is to disable a presolve step automatically. Can we address it like effective_bound_free_variables?
There was a problem hiding this comment.
added effective_eliminate_free_variables like the effective_bound_free_variables
| impl_->rhs_dirty = true; | ||
| } | ||
|
|
||
| #ifdef DUAL_SIMPLEX_INSTANTIATE_DOUBLE |
There was a problem hiding this comment.
Do we really need #ifdef here?
| const i_t problem_rows = user_problem.original_num_rows; | ||
| std::vector<i_t> row_nz(problem_rows, 0); | ||
| for (i_t j = 0; j < user_problem.num_cols; ++j) { | ||
| for (i_t p = A.col_start[j]; p < A.col_start[j + 1]; ++p) { |
There was a problem hiding this comment.
It looks the same as before. Can we undo the change here?
There was a problem hiding this comment.
It was addressing a previous review
#1979 (comment)
| cone_head_bound_t<i_t, f_t> bound; | ||
| bound.head_col = head; | ||
| // A is CSC, so the head's own column already lists every row it appears in. | ||
| for (i_t p = A.col_start[head]; p < A.col_start[head + 1]; ++p) { |
There was a problem hiding this comment.
it was addressing a previous review
#1979 (comment)
|
/ok to test 6562707 |
Description
follows #1941 and #1913
Barrier sequence solves (update_linear_objective and update_rhs) now cover second-order cone programs built from quadratic constraints.
The cache stores the expansion: the pre-expansion sizes, the column permutation, the cone start before and after inequality slacks are inserted, the appended cone-row right-hand sides, and the rows that proved a cone head nonnegative. The crush replays that map, the slack shift, and the free-variable equality substitution so the update lands on the cached barrier problem. The quadratic-constraint constant is kept in the cone-row tail and is not overwritten.
A warm reuse restores the cone block to the initial diagonal before the first factorization. Reuse is refused when cone variables were aliased, and the cone layout must match the cached one.
Issue
Checklist