Bug fix of dual init point computation in barrier#1577
Conversation
Signed-off-by: yuwenchen95 <yuwchen@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChangesThe barrier solver adds SOC-aware interior initialization and the Sturm/SeDuMi mu-based strategy. Diagonal preprocessing stores Barrier initialization
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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/barrier/barrier.cu (1)
2187-2236: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSOC initial-point branches are unreachable for cone problems
data.has_cones()forcesinit_strategyto2, and that branch returns immediately, so the laterhas_cones()/has_socpath ininitial_point()never runs for SOCPs. That makes the new SOC interior-enforcement helper and the sparse-SOCaug_sizepath dead code here, and it also silently ignores any explicitbarrier_dual_initial_point = 0/1setting on cone problems.🤖 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/barrier.cu` around lines 2187 - 2236, Update initial_point’s init_strategy selection and early-return flow so cone problems do not unconditionally force strategy 2. Preserve explicit barrier_dual_initial_point values 0/1 for cone problems, and route those cases through the existing SOC initialization/interior-enforcement and sparse-SOC aug_size logic; retain the mu-based strategy 2 behavior when selected.Source: Path instructions
python/cuopt_server/cuopt_server/tests/test_lp.py (1)
163-172: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd
dual_initial_point=2to the test matrix. The current@pytest.mark.parametrizelist covers-1,0, and1, but not the new2value, so the mu-based path still isn’t exercised.🤖 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 `@python/cuopt_server/cuopt_server/tests/test_lp.py` around lines 163 - 172, Update the pytest parameterization for test_barrier_solver_options to include dual_initial_point=2 alongside the existing -1, 0, and 1 values, ensuring the mu-based path is exercised.
🤖 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/barrier/barrier.cu`:
- Around line 2187-2236: Update initial_point’s init_strategy selection and
early-return flow so cone problems do not unconditionally force strategy 2.
Preserve explicit barrier_dual_initial_point values 0/1 for cone problems, and
route those cases through the existing SOC initialization/interior-enforcement
and sparse-SOC aug_size logic; retain the mu-based strategy 2 behavior when
selected.
In `@python/cuopt_server/cuopt_server/tests/test_lp.py`:
- Around line 163-172: Update the pytest parameterization for
test_barrier_solver_options to include dual_initial_point=2 alongside the
existing -1, 0, and 1 values, ensuring the mu-based path is exercised.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7b1686c5-e726-4b2a-8619-e97d44323b58
📒 Files selected for processing (5)
cpp/src/barrier/barrier.cucpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/math_optimization/solver_settings.cupython/cuopt_server/cuopt_server/tests/test_lp.pypython/cuopt_server/cuopt_server/utils/linear_programming/data_definition.py
Signed-off-by: yuwenchen95 <yuwchen@nvidia.com>
130b9a4 to
8e29f92
Compare
CI Test Summary1 failed · 30 passed · 0 skipped |
Description
Extend barrier initial-point handling for SOCP and fix augmented-path KKT assembly at initialization.
ensure_initial_point_interiorto push linear variables into the nonnegative orthant and SOC blocks into strict Lorentz cone interior; apply toxandzat the end ofinitial_point.diag.multiply_scalar(-1.0)so the first-call augmented build produces-(Q+D)on the primal block, matchingaugmented_multiplyand the refactor path.max(n+m, device_augmented.n)for sparse SOC expansion, zero-padding extra entries.d_diag_from hostdiag; skip IR for cone problems and direct-free linear variables where operator/factorization would disagree.inv_sqrt_diagmember and initialization.Issue
Checklist