Repository navigation
Project barrier variables back to original space in all scenarios - #2230
Conversation
Signed-off-by: yuwenchen95 <yuwchen@nvidia.com>
Signed-off-by: yuwenchen95 <yuwchen@nvidia.com>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Project barrier solutions and validate output dimensions cpp/src/pdlp/solve.cu, cpp/tests/socp/general_quadratic_test.cu |
run_barrier projects the solution without an optimal-status guard. The test checks primal and reduced-cost vector sizes for time-limit and iteration-limit termination. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Medium
Possibly related PRs
- NVIDIA/cuopt#2208: Fixes oversized primal output for non-optimal cone solves. This PR instead projects the solution for every solver status.
Suggested labels: improvement, barrier
Suggested reviewers: chris-maes
Merge Risk: 
5469b
-
The barrier result now preserves model-sized output vectors for limit terminations without an established merge-blocking defect.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title clearly and concisely describes the main change: projecting barrier variables back to the original model space for all solver statuses. Description check The description directly explains the dimension-mismatch issue, the code change, the affected non-optimal statuses, and the expected behavior. Linked Issues check Issue #2187requires that a time-limit result for an SOCP model return a primal in the four original variables, or report that no usable primal exists. It must not raise a dimension-mismatch exception…Out of Scope Changes check The whole-PR diff contains two changes. The first removes the OPTIMAL-only guard around the barrier projection in cpp/src/pdlp/solve.cu, a one-line-level change of +1/-3 lines. The second adds one reg… Full details: Linked Issues check
-
Explanation
Issue
#2187requires that a time-limit result for an SOCP model return a primal in the four original variables, or report that no usable primal exists. It must not raise a dimension-mismatch exception. The diff from merge base to head changes cpp/src/pdlp/solve.cu. The call to barrier::project_barrier_solution_to_model_variables now runs for every simplex::lp_status_t, not only OPTIMAL. The guard was removed, so non-optimal barrier results are mapped to model variables. The added test socp_limits_return_model_sized_primal in cpp/tests/socp/general_quadratic_test.cu runs barrier with time_limit = 0.0 and with iteration_limit = 3. It expects TimeLimit and IterationLimit terminations. It checks that both the primal solution and the reduced-cost vector have n entries, where n is the model variable count. This covers the reported failure and the expected behavior in#2187. Limitation: I did not execute the test. The PASS rests on the source diff and the test code.
Full details: Out of Scope Changes check
-
Explanation
The whole-PR diff contains two changes. The first removes the OPTIMAL-only guard around the barrier projection in cpp/src/pdlp/solve.cu, a one-line-level change of +1/-3 lines. The second adds one regression test in cpp/tests/socp/general_quadratic_test.cu that exercises the time-limit and iteration-limit paths for barrier. Both changes connect directly to
#2187. No unrelated files or hunks appear in the diff. The PR description states that models without quadratic constraints are unaffected because the projection does nothing without cone expansion. This statement is consistent with the change and does not add out-of-scope behavior.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
CI Test Summary✅ All 21 test job(s) passed. |
Signed-off-by: yuwenchen95 <yuwchen@nvidia.com>
|
/ok to test |
Signed-off-by: yuwenchen95 <yuwchen@nvidia.com>
|
/merge |
Description
A barrier solve with quadratic constraints that hit the time or iteration limit returned
its primal and reduced-cost vectors in the internal cone-expanded layout (e.g. 10 entries
for a 4-variable model). In Python this raised
ValueError: variable_names length (4) does not match solution size (10).run_barriermapped the solution back to model variables only when the status wasOPTIMAL. It now does this for every status, so a time or iteration limit returnsmodel-sized vectors. The primal is all zeros, because barrier fills the solution only at
optimality. Problems without quadratic constraints are unaffected, since the mapping does
nothing when there is no cone expansion.
Issue
Closes #2187