Add iterative refinement to the GPU ADAT solve path - #1680
Conversation
📝 WalkthroughWalkthroughThe benchmark script now discovers compressed MPS files. The barrier solver adds device-vector ADAT multiplication and optional GMRES iterative refinement for eligible non-augmented solves. ChangesBenchmark input discovery
ADAT iterative refinement
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cpp/src/barrier/barrier.cu (1)
1721-1735: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReuse a persistent ADAT workspace.
Line 1727 allocates a device vector for every GMRES matrix-vector product. GMRES invokes this callback repeatedly. Reuse an iteration-owned workspace, such as
d_u_, to avoid repeated device allocation in the solve hot path.Proposed refactor
- rmm::device_uvector<f_t> u(n, stream_view_); + auto& u = d_u_; cusparse_view_.transpose_spmv(1.0, y, 0.0, u);🤖 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 1721 - 1735, Update gpu_adat_multiply_simple to reuse the iteration-owned persistent workspace d_u_ instead of allocating a new device_uvector<f_t> on each call. Ensure d_u_ is sized and valid for n before transpose_spmv and subsequent elementwise multiplication, while preserving the existing computation and synchronization behavior.
🤖 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.
Nitpick comments:
In `@cpp/src/barrier/barrier.cu`:
- Around line 1721-1735: Update gpu_adat_multiply_simple to reuse the
iteration-owned persistent workspace d_u_ instead of allocating a new
device_uvector<f_t> on each call. Ensure d_u_ is sized and valid for n before
transpose_spmv and subsequent elementwise multiplication, while preserving the
existing computation and synchronization behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3c1bed04-2642-4f5d-83ce-d3dd8a368a02
📒 Files selected for processing (2)
benchmarks/linear_programming/run_mps_files.shcpp/src/barrier/barrier.cu
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
/ok to test 8580c87 |
@rg20, there was an error processing your request: See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/2/ |
|
/ok to test 86ff783 |
CI Test Summary✅ All 31 test job(s) passed. |
|
/merge |
2 participants
Description
Adds iterative refinement to the GPU ADAT (Schur-complement) solve path in the barrier solver
Changes:
Motivation:
As the barrier parameter shrinks, D spans huge magnitude ranges and the direct Cholesky solve alone can degrade. Refining the ADAT solution improves robustness on ill-conditioned LP/QP instances without affecting the SOCP path (which does not use the Schur-complement formulation).
Benchmarks:
Acknowledgment: This improvement was proposed by the Hiverge AI discovery engine with experiments by @kerry-hiverge.
Checklist