Bug fix: Insert barrier slack columns in front of the cone block for range rows - #2029
yuwenchen95 wants to merge 2 commits into
Conversation
Signed-off-by: yuwenchen95 <yuwchen@nvidia.com>
yuwenchen95
added
bug
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: NVIDIA/cuopt/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughPresolve initializes the quadratic matrix before constraint conversion. Less-than and range rows use shared slack insertion before cone variables, with updates to the constraint and quadratic matrices. Two barrier solver tests cover SOC problems with linear and quadratic objectives. ChangesSOC slack-column conversion
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The reviewed presolve changes preserve the quadratic terms and ranged-row feasible intervals. No actionable merge-blocking issue was identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
CI Test Summary1 failed · 31 passed · 0 skipped
|
| return 0; | ||
| } | ||
|
|
||
| // Insert slacks and keep [linear | slacks | cone] layout |
There was a problem hiding this comment.
I'm worried about this layout for the future: MIP relies on slacks being last.
We don't have an MISOCP solver now. But I think we have a conflict where both the MIP solver and the cone solver want certain variables to appear last.
|
|
||
| // Insert slacks and keep [linear | slacks | cone] layout | ||
| template <typename i_t, typename f_t> | ||
| static void insert_slack_columns(lp_problem_t<i_t, f_t>& problem, |
There was a problem hiding this comment.
Please add a comment here explaining what this code is doing mathematically.
What are the inputs? What is rows? What is coefficient?
There was a problem hiding this comment.
Done. Add comments for input parameters.
| problem.Q.m = problem.num_cols; | ||
| problem.Q.n = problem.num_cols; | ||
| problem.Q.nz_max = user_problem.Q_values.size(); | ||
| problem.Q.row_start.assign(user_problem.Q_offsets.begin(), |
There was a problem hiding this comment.
What about problem.Q.row_start from user_problem.num_cols to problem.num_cols?
There was a problem hiding this comment.
This is just Q copy from user_problem to problem. Maybe I'd better change lines
problem.Q.m = problem.num_cols; problem.Q.n = problem.num_cols; to problem.Q.m = user_problem.num_cols; problem.Q.n = user_problem.num_cols; for clearer purpose.
| problem.upper.insert(problem.upper.begin() + insert_at, upper.begin(), upper.end()); | ||
|
|
||
| if (problem.Q.n > 0) { | ||
| // Slacks have empty Q rows and columns: add the empty rows, then shift the column indices |
There was a problem hiding this comment.
Does it make sense to put the slacks into the middle? If we can have quadratic objective terms on cone variables, we know have a gap in the matrix. If we put slacks at the end we could use a smaller representation for Q, since we know that slacks can't participate in Q.
…perf
Brings in upstream main up to the PR's base. Conflict resolutions:
- cusparse_view.cu: keep the on-device CSR build; port the device-side
alg2_beta_bug_possible check so all three constructors set
beta_bug_possible_{,transpose_} for the upstream ALG1/ALG2 selection.
- iterative_refinement.hpp: keep the fixed_point/gmres method argument,
drop the default tol as upstream did.
- barrier.cu: keep the IR method enum, adopt upstream's
use_high_accuracy_ir tolerance and switch.
- pdlp/solve.cu: keep barrier_relative_complementarity_tol.
- grpc codegen: regenerated from the merged field_registry.yaml.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: yuwenchen95 <yuwchen@nvidia.com>
3 participants
Description
Keep [linear | slacks | cone] layout, which is missing for range row in presolve.