Replaced SpMV with SpMVOp in PDHG for double precision by Bubullzz · Pull Request #1226 · NVIDIA/cuopt

Bubullzz · 2026-05-15T10:52:34Z

Description

Replaced SpMV calls with SpMVOp calls in PDHG in order to accelerate compute. These changes are only available for double precision. Benchmarks on B200 full Mittleman dataset with concurrent strategy gave a 1.09x speedup.

follow-up on #989

Issue

closes #9

…bnvjitlink-dev manually

…rs wrappers

copy-pr-bot · 2026-05-15T10:52:38Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

coderabbitai · 2026-05-15T10:57:28Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds CUDA 13.2+ cuSPARSE SpMVOp acceleration for PDLP: RAII wrappers and runtime probe, cusparse_view_t integration with SpMVOp workspace and plan creation, PDHG helpers that dispatch to plans or fall back, plus build and conda dependency updates.

Changes

cuSPARSE SpMVOp Acceleration

Layer / File(s)	Summary
SpMVOp wrapper types and infrastructure `cpp/src/pdlp/cusparse_view.hpp`, `cpp/src/pdlp/cusparse_view.cu`	Introduces `CUDA_VER_13_2_UP` and defines `cusparse_spmvop_descr_wrapper_t` and `cusparse_spmvop_plan_wrapper_t` RAII wrappers with lifecycle management, `create()` methods, and conversion operators; implements wrappers in the .cu file.
cusparse_view_t SpMVOp support and plan initialization `cpp/src/pdlp/cusparse_view.hpp`, `cpp/src/pdlp/cusparse_view.cu`	Adds SpMVOp scratch buffers (`buffer_non_transpose_spmvop`, `buffer_transpose_spmvop`), descriptor/plan members declared for correct destruction order, constructor initializer updates, runtime `is_cusparse_runtime_spmvop_supported()` probe, and `create_spmv_op_plans(bool is_reflected)` to build plans for `A_T` and optionally `A`.
PDHG solver SpMV helper methods and integration `cpp/src/pdlp/pdhg.hpp`, `cpp/src/pdlp/pdhg.cu`	Adds private helpers `spmvop_At_y()`, `spmvop_A_x()`, and `my_spmvop(...)` that dispatch between cuSPARSE SpMVOp plans (when available) and `cusparsespmv` fallback; integrates these into `compute_At_y()` and `compute_A_x()` non-mixed-precision paths.
Solver initialization, build configuration, and dependencies `cpp/src/pdlp/pdlp.cu`, `cpp/CMakeLists.txt`, `dependencies.yaml`, `conda/environments/*`	Calls `create_spmv_op_plans()` during `pdlp_solver_t::run_solver` initialization for double-precision non-batch runs when mixed precision is disabled; adds `CUSPARSE_ENABLE_EXPERIMENTAL_API` to `cuopt` target compile definitions; adds `libnvjitlink-dev` to CUDA conda dependency lists and environment YAMLs.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

tmckayus

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Replaced SpMV with SpMVOp in PDHG for double precision' accurately summarizes the main objective of the changeset, which is replacing SpMV calls with SpMVOp for performance improvement in PDHG.
Description check	✅ Passed	The description explains the purpose (SpMV to SpMVOp replacement for acceleration), scope (double precision only), provides performance metrics (1.09x speedup), and references related issues, demonstrating clear relevance to the changeset.
Linked Issues check	✅ Passed	The changes implement SpMV acceleration for PDHG through SpMVOp support (CUDA 13.2+), wrapper classes, integration into pdhg_solver, and runtime detection, directly addressing the objective to improve spmv acceleration for pdlp in issue `#9`.
Out of Scope Changes check	✅ Passed	All changes are within scope: SpMVOp wrappers and integration for PDHG acceleration, CUDA 13.2+ gating, dependency additions for nvjitlink, and backward compatibility via runtime checks and fallbacks.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

dependencies.yaml (1)

670-676: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Change line 670 selector from cuda: "13.2" to cuda: "13.1" to resolve conflicting CUDA version mappings.

The matrix currently has duplicate entries for cuda: "13.2" with conflicting package pins (13.1 and 13.2), which breaks dependency resolution for CUDA 13.2 environments.

🔧 Proposed fix

       - output_types: conda
         matrices:
-          - matrix:
-              cuda: "13.2"
+          - matrix:
+              cuda: "13.1"
             packages:
               - cuda-version=13.1
           - matrix:
               cuda: "13.2"
             packages:
               - cuda-version=13.2

🤖 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 `@dependencies.yaml` around lines 670 - 676, Update the YAML matrix entry so
the CUDA selector matches its package pin: change the first occurrence of the
CUDA matrix key currently set as cuda: "13.2" (the block that lists packages: -
cuda-version=13.1) to cuda: "13.1" so the selector and the pinned package agree;
ensure you edit the matrix entry containing the keys "cuda" and "packages" (the
block pairing cuda-version=13.1) rather than the block already pinned to
cuda-version=13.2.

🧹 Nitpick comments (1)

cpp/src/pdlp/cusparse_view.hpp (1)
23-23: 💤 Low value

Macro naming should use CUOPT_ prefix.

The macro CUDA_VER_13_2_UP does not follow the project naming convention. As per coding guidelines, project macros should use SCREAMING_SNAKE_CASE with a CUOPT_ prefix.
♻️ Suggested rename
-#define CUDA_VER_13_2_UP (CUDART_VERSION >= 13020)
+#define CUOPT_CUDA_VER_13_2_UP (CUDART_VERSION >= 13020)
Then update all uses of this macro in this file and dependent files.
As per coding guidelines, project macros should use SCREAMING_SNAKE_CASE with CUOPT_ prefix.
🤖 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/pdlp/cusparse_view.hpp` at line 23, Rename the macro CUDA_VER_13_2_UP
to follow project convention (e.g., CUOPT_CUDA_VER_13_2_UP) and update its
definition in cusparse_view.hpp; then replace all references to CUDA_VER_13_2_UP
in this file and any dependent files with the new symbol CUOPT_CUDA_VER_13_2_UP
to keep naming consistent with project-wide macros.

🤖 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.

Inline comments:
In `@cpp/src/pdlp/cusparse_view.cu`:
- Around line 1280-1338: The method create_spmv_op_plans is templated on f_t but
always uses CUDA_R_64F, so add a compile-time guard to only instantiate the
double-precision path: wrap the existing CUDA_VER_13_2_UP body in an if
constexpr (std::is_same_v<f_t, double>) { ... } block so float instantiations
skip this code; ensure <type_traits> is available for std::is_same_v and
reference the symbols used (create_spmv_op_plans, f_t, CUDA_R_64F, A_T, A,
spmv_op_descr_A_t_, spmv_op_descr_A_, spmv_op_plan_A_t_, spmv_op_plan_A_) when
making the change.

In `@cpp/src/pdlp/pdhg.hpp`:
- Around line 98-101: Remove the unused dead declaration my_spmvop(f_t* alpha,
f_t* A, f_t* x, f_t* beta, f_t* y, f_t* result) from the pdhg.hpp header: delete
the entire method prototype so only the actual SPMV methods (spmvop_At_y and
spmvop_A_x) remain; ensure no other code references my_spmvop (rename or remove
any forward declarations) and recompile to verify no missing-symbol errors.

---

Outside diff comments:
In `@dependencies.yaml`:
- Around line 670-676: Update the YAML matrix entry so the CUDA selector matches
its package pin: change the first occurrence of the CUDA matrix key currently
set as cuda: "13.2" (the block that lists packages: - cuda-version=13.1) to
cuda: "13.1" so the selector and the pinned package agree; ensure you edit the
matrix entry containing the keys "cuda" and "packages" (the block pairing
cuda-version=13.1) rather than the block already pinned to cuda-version=13.2.

---

Nitpick comments:
In `@cpp/src/pdlp/cusparse_view.hpp`:
- Line 23: Rename the macro CUDA_VER_13_2_UP to follow project convention (e.g.,
CUOPT_CUDA_VER_13_2_UP) and update its definition in cusparse_view.hpp; then
replace all references to CUDA_VER_13_2_UP in this file and any dependent files
with the new symbol CUOPT_CUDA_VER_13_2_UP to keep naming consistent with
project-wide macros.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 189500be-24af-4f85-953b-6e3d3f4010f0

📥 Commits

Reviewing files that changed from the base of the PR and between 32af987 and a85b10f.

📒 Files selected for processing (7)

cpp/CMakeLists.txt
cpp/src/pdlp/cusparse_view.cu
cpp/src/pdlp/cusparse_view.hpp
cpp/src/pdlp/pdhg.cu
cpp/src/pdlp/pdhg.hpp
cpp/src/pdlp/pdlp.cu
dependencies.yaml

Bubullzz · 2026-05-15T10:59:48Z

/ok to test 1880f23

Bubullzz · 2026-05-18T12:28:50Z

/ok to test 36c41ca

Bubullzz · 2026-05-18T12:33:52Z

/ok to test e1890ef

Bubullzz · 2026-05-18T12:40:16Z

/ok to test c907d93

Bubullzz · 2026-05-18T12:41:11Z

/ok to test 99f455d

Bubullzz · 2026-05-18T12:42:00Z

/ok to test 45efb50

Bubullzz · 2026-05-18T14:28:04Z

/ok to test 152fc19

Bubullzz · 2026-05-18T16:51:03Z

/ok to test f08facd

Bubullzz · 2026-05-18T20:40:23Z

/ok to test 5cce25f

Bubullzz · 2026-05-19T10:09:07Z

/ok to test 5cce25f

copy-pr-bot · 2026-05-19T10:09:11Z

/ok to test 5cce25f

@Bubullzz, there was an error processing your request: E2

See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/2/

Bubullzz · 2026-05-19T10:13:36Z

/ok to test 1ebd68c

Bubullzz added 12 commits May 15, 2026 12:45

replaced SpMV calls with SpMVOp calls in compute_At_y and compute_A_x

cccace9

small fix for full OK tests

dd12e1a

coding style and updated dependencies. still need to conda install li…

2439f92

…bnvjitlink-dev manually

updated dependencies.yaml

86be5b2

added comments and removed a useless set_stream

81b8f77

moved create_spmv_op_plans and added libnvjitlink-dev

c5f8db2

added guards for spmvop compilation

583da59

added destructors

4f041f1

moved buffer_spmvop to allow <13.2 compilation

2a1360d

removed cusparse_view destructor and added spmvop plamd and descripto…

96a524a

…rs wrappers

added cusparseSetStream for safety

6b8e78a

made compute At_y and compute A_x spmv choosing more readable

a85b10f

Bubullzz requested review from a team as code owners May 15, 2026 10:52

Bubullzz requested review from KyleFromNVIDIA, hlinsen, nguidotti and tmckayus May 15, 2026 10:52

Bubullzz changed the title ~~Spmvop 3~~ Replaced SpMV with SpMVOp in PDHG for double precision May 15, 2026

Bubullzz mentioned this pull request May 15, 2026

[BROKEN PR] Replaced SpMV with SpMVOp in PDHG for double precision #989

Closed

coderabbitai Bot reviewed May 15, 2026

View reviewed changes

Comment thread cpp/src/pdlp/cusparse_view.cu

Comment thread cpp/src/pdlp/pdhg.hpp Outdated

Bubullzz added 2 commits May 15, 2026 12:58

update broken dependency

68922b0

pre-commit run

1880f23

Bubullzz added non-breaking Introduces a non-breaking change improvement Improves an existing functionality pdlp labels May 15, 2026

Bubullzz added the P2 label May 15, 2026

mlubin reviewed May 15, 2026

View reviewed changes

Comment thread cpp/src/pdlp/cusparse_view.hpp Outdated

mlubin added this to the 26.06 milestone May 15, 2026

Bubullzz added 3 commits May 18, 2026 14:06

added dlsym checks to SpMVOp path

f69f77f

removed raw buffers/pointers and added doc

238adac

small fix error name

36c41ca

style

e1890ef

added guards for only double instantiation of spmvop

c907d93

removed unused my_spmv_op

99f455d

Merge branch 'main' into spmvop_3

45efb50

made all spmvop function dlsym resolved

152fc19

Merge branch 'main' into spmvop_3

f08facd

Bubullzz closed this May 18, 2026

Bubullzz reopened this May 18, 2026

Merge branch 'main' into spmvop_3

5cce25f

Merge branch 'main' into spmvop_3

1ebd68c

Conversation

Bubullzz commented May 15, 2026

Description

Issue

Uh oh!

copy-pr-bot Bot commented May 15, 2026

Uh oh!

coderabbitai Bot commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Bubullzz commented May 15, 2026

Uh oh!

Uh oh!

Bubullzz commented May 18, 2026

Uh oh!

Bubullzz commented May 18, 2026

Uh oh!

Bubullzz commented May 18, 2026

Uh oh!

Bubullzz commented May 18, 2026

Uh oh!

Bubullzz commented May 18, 2026

Uh oh!

Bubullzz commented May 18, 2026

Uh oh!

Bubullzz commented May 18, 2026

Uh oh!

Bubullzz commented May 18, 2026

Uh oh!

Bubullzz commented May 19, 2026

Uh oh!

copy-pr-bot Bot commented May 19, 2026

Uh oh!

Bubullzz commented May 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented May 15, 2026 •

edited

Loading