Repository navigation
feat(hipsparse): add generic SpGEAM API for sparse matrix-matrix addition - #9648
Merged
kliegeois merged 29 commits intoAug 10, 2026
Merged
Conversation
Document the hipsparseSpGEAM generic API (createDescr, destroyDescr, bufferSize, nnz, and compute entry points), the hipsparseSpGEAMDescr_t type, and the hipsparseSpGEAMAlg_t enum in the API reference, and add the routines to the sparse generic support matrix. Co-authored-by: Cursor <cursoragent@cursor.com>
Previously the SpGEAM CSR test built B from A's exact sparsity pattern, so C always matched A's pattern and the pattern-merge path (columns present in only one operand) and the sorted-output-column guarantee were never validated. Generate B as an independent m x n matrix instead so these paths are covered against the host csrgeam reference. Co-authored-by: Cursor <cursoragent@cursor.com>
Add quick-category configs with alpha and/or beta equal to zero. The sparsity pattern of C is the union of A and B and is independent of the scalar values on both the rocSPARSE and cuSPARSE backends, so the union host reference validates these cases on both backends. Co-authored-by: Cursor <cursoragent@cursor.com>
hipsparseSpGEAM only supports C = alpha * A + beta * B with non-transpose operations on CSR matrices, as documented, but a transpose operation was silently accepted and forwarded, producing an incorrect result instead of an error. Validate the operations and matrix formats up front in both the rocSPARSE and cuSPARSE backends and return HIPSPARSE_STATUS_NOT_SUPPORTED for transpose operations or non-CSR formats. Extend the bad-arg test to cover these cases. Co-authored-by: Cursor <cursoragent@cursor.com>
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
Format the SpGEAM sources with the pinned clang-format (v18.1.4) so the pre-commit check passes, and update the copyright year in hipsparse.h to 2026 to reflect the SpGEAM header addition. Co-authored-by: Cursor <cursoragent@cursor.com>
jsandham
reviewed
Jul 21, 2026
jsandham
reviewed
Jul 21, 2026
The Math CI static-analysis format check uses clang-format 13. Reverting the amd_detail/utility.h Entry() assignment alignment to the develop (clang-format 13) style; the earlier whole-file reformat had used a newer clang-format that aligns it differently and broke the format check. Co-authored-by: Cursor <cursoragent@cursor.com>
jsandham
reviewed
Jul 21, 2026
jsandham
reviewed
Jul 21, 2026
jsandham
reviewed
Jul 21, 2026
- changelog: move the SpGEAM entry to the unreleased hipSPARSE 4.7.0 section (4.6.0/ROCm 7.14 is already released) - docs: reword the hipsparseSpGEAM_nnz brief per review suggestion - tests: add the SpGEAM test to the cuda/12.8 set so the cuSPARSE backend CI exercises it - tests: expand quick, pre_checkin, and nightly coverage (mixed index bases, complex scalars, medium random matrices, and large real/random matrices) to use the available test-time budget Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
Once cuda compilation is fixed, running the tests I get: [==========] Running 96 tests from 2 test suites. [ FATAL ] hipsparse-test TERMINATED BY SIGSEGV during test: quick/spgeam_csr.generic/f32_r_325_174_2_0_n1p5_0_0b_0b_0b |
jsandham
reviewed
Jul 21, 2026
jsandham
reviewed
Jul 21, 2026
jsandham
reviewed
Jul 21, 2026
jsandham
reviewed
Jul 21, 2026
jsandham
reviewed
Jul 21, 2026
jsandham
reviewed
Jul 21, 2026
jsandham
reviewed
Jul 21, 2026
- nvidia backend: cast matA/matB to cusparseSpMatDescr_t (non-const) in the SpGEAM buffer size, nnz, and compute passthroughs to match the cusparseSpGEAM API and fix the CUDA build - tests: remove the cuda/12.8 SpGEAM test and wiring; cusparseSpGEAM was only introduced in CUDA 13.3 so it cannot be exercised on the 12.8 backend Co-authored-by: Cursor <cursoragent@cursor.com>
jsandham
reviewed
Jul 21, 2026
dso-amd
reviewed
Jul 22, 2026
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #9648 +/- ##
===========================================
- Coverage 68.88% 68.86% -0.02%
===========================================
Files 2765 2763 -2
Lines 455315 454959 -356
Branches 67142 67063 -79
===========================================
- Hits 313637 313304 -333
+ Misses 118764 118745 -19
+ Partials 22914 22910 -4
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
…backend rocSPARSE SpGEAM computes the union sparsity pattern of A and B regardless of the scalar values, whereas cuSPARSE collapses the degenerate zero-scalar cases: alpha == 0 -> C = beta*B (pattern of B), beta == 0 -> C = alpha*A (pattern of A), and both zero -> empty C. Handle these cases in the hipSPARSE SpGEAM wrapper on the ROCm backend using the new rocsparse_spmat_scale primitive instead of rocsparse_spgeam: the nnz step sets C's nonzero count and copies the surviving operand's row offsets (or an empty pattern), and the compute step copies the surviving column indices and writes C's values as the surviving scalar times the operand's values via rocsparse_spmat_scale. The scalar's host/device memory space is described on a rocsparse_dnvec_descr_create_scalar descriptor, so device pointer mode works too. The zero-scalar cases were previously excluded from the rocm test suite as known bugs; that exclusion is removed. 148 spgeam tests pass on gfx1201 (previously 96), including alpha/beta zero edge cases in host and device pointer mode. Co-authored-by: Cursor <cursoragent@cursor.com>
The zero-scalar cases are no longer a known bug on the rocSPARSE backend; the hipSPARSE wrapper now reproduces the cuSPARSE collapse behavior via rocsparse_spmat_scale. Update the stale comments in both backend test yamls. Co-authored-by: Cursor <cursoragent@cursor.com>
ntrost57
approved these changes
Aug 5, 2026
Contributor
|
I suggest adding a feature flag to keep QA happy. |
…eature flag - Add \brief summaries to hipsparseSpGEAM_createDescr and hipsparseSpGEAM_destroyDescr (review feedback). - Gate the generic SpGEAM API behind a build-time feature flag (HIPSPARSE_ENABLE_SPGEAM cmake option -> HIPSPARSE_WITH_SPGEAM, default ON), registered in hipsparse-config.h.in and enforced by the feature-flag guardrail. The public API (functions in hipsparse_spgeam.h, the hipsparseSpGEAMDescr_t / hipsparseSpGEAMAlg_t types), both backend implementations, the alg converters, and the clients (test, unique_ptr helper, sample) are all guarded so the feature can be disabled cleanly for QA. Verified on gfx1201: with the flag ON all 148 spgeam tests pass; with the flag OFF the library and clients build with the SpGEAM surface removed. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
@amontoison good point, done, thanks! |
amontoison
approved these changes
Aug 5, 2026
Merged
2 tasks
The generic SpGEAM API lands in hipSPARSE 5.1, so move the HIPSPARSE_WITH_SPGEAM #cmakedefine under a "hipSPARSE 5.1" group in hipsparse-config.h.in and move its CHANGELOG entry to a new (Unreleased) hipSPARSE 5.1.0 section. JIRA ID : AISPARSE-586 Co-authored-by: Cursor <cursoragent@cursor.com>
JIRA ID : AISPARSE-586 Co-authored-by: Cursor <cursoragent@cursor.com>
math-ci runs clang-format 19, which rewrites the SpGEAM preprocessor
conditionals from "#if(" to "#if (", while the repo pre-commit hook is
pinned to clang-format 18 (SpaceBeforeParens: Never), which keeps "#if(".
The two gates are mutually exclusive on these lines, so wrap the affected
directives in "// clang-format off/on" with an explanatory comment so both
formatters accept them.
Co-authored-by: Cursor <cursoragent@cursor.com>
kliegeois
added a commit
that referenced
this pull request
Aug 6, 2026
…MAT_SCALE feature flag (#10417) ## Summary PR #9729 added the generic `rocsparse_spmat_scale` routine and its supporting auxiliary creator `rocsparse_dnvec_descr_create_scalar`, but — unlike the analogous hipSPARSE generic SpGEAM API added in #9648 (gated by `HIPSPARSE_ENABLE_SPGEAM` → `HIPSPARSE_WITH_SPGEAM`) — it did not add a build-time feature flag. As a result the new API was unconditionally compiled and exported and could not be turned off. This PR adds the missing gate, mirroring the existing rocSPARSE feature flags (`ROCSPARSE_WITH_ILDLT0`, `ROCSPARSE_WITH_HANDLE_CREATE`, `ROCSPARSE_WITH_ILU0_BOOST_SIGN`, …) and the hipSPARSE SpGEAM precedent. ## Changes - Add the `ROCSPARSE_ENABLE_SPMAT_SCALE` CMake option (default `ON`) in `projects/rocsparse/CMakeLists.txt`. - Map it to the `ROCSPARSE_WITH_SPMAT_SCALE` macro in `library/CMakeLists.txt` and register the `#cmakedefine` in `rocsparse-config.h.in`. - Gate the public declarations: the `rocsparse_spmat_scale.h` include in `internal/rocsparse-generic.h`, and the `rocsparse_dnvec_descr_create_scalar` declaration in `rocsparse-auxiliary.h`. - Gate the library implementations: the body of `src/extra/rocsparse_spmat_scale.cpp` and the `rocsparse_dnvec_descr_create_scalar` definition in `src/auxiliary/rocsparse_auxiliary.cpp`. - Gate the client side: the `spmat_scale` test enum entry (via a `ROCSPARSE_FOREACH_TEST_ENUM_SPMAT_SCALE` macro), the test/testing source registration in `clients/tests/CMakeLists.txt`, and the `example_spmat_scale` sample registration in `clients/samples/CMakeLists.txt`. - CHANGELOG updated. When the option is `OFF`, `rocsparse_spmat_scale` and `rocsparse_dnvec_descr_create_scalar` are neither exported nor compiled, and the associated tests/sample are dropped. The internal, always-safe additions from #9729 (the `pointer_mode` member on the dense-vector descriptor and the explicit-pointer-mode `scale_array` overload) are left unconditional, as they are harmless when unused. ## Validation Because this is a build-configuration / preprocessor-gating change, it was validated as follows (full compile + GPU test to run in CI, as with #9729): - The repo feature-flag guardrail (`cmake/CheckFeatureFlags.cmake`) passes: `ROCSPARSE_WITH_SPMAT_SCALE` is registered in the config template and every public header that uses it (`rocsparse-generic.h`, `rocsparse-auxiliary.h`) includes `rocsparse-config.h`. - `configure_file` renders `#define ROCSPARSE_WITH_SPMAT_SCALE` when the option is `ON` and `#undef` when `OFF`. - `#if`/`#endif` guards are balanced in all edited translation units. ## Test plan - [ ] Build with `ROCSPARSE_ENABLE_SPMAT_SCALE=ON` (default); `rocsparse-test --gtest_filter='*spmat_scale*'` passes and the `example_spmat_scale` sample builds. - [ ] Build with `-DROCSPARSE_ENABLE_SPMAT_SCALE=OFF`; the library, clients, and samples build with the routine, its helper, its tests, and its sample compiled out. JIRA ID : AISPARSE-737 Made with [Cursor](https://cursor.com) --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: jsandham <jsandham@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Keep the changelog diff against develop purely additive: the SpGEAM PR had collapsed one of the two blank lines preceding the (Unreleased) 4.8.0 header. Restore it so only the new SpGEAM entry is added and no existing line is removed. Co-authored-by: Cursor <cursoragent@cursor.com>
… in SpGEAM header The generic SpGEAM header lives in a subdirectory (internal/generic) and included "hipsparse-config.h" with a bare path. That resolved in the hipSPARSE build (which adds -I <build>/include/hipsparse) but not for downstream consumers such as hipSPARSELt, which only add the include root (-I <prefix>/include) and thus failed with 'hipsparse-config.h file not found'. Use "hipsparse/hipsparse-config.h" so it resolves via the include root in both cases. Co-authored-by: Cursor <cursoragent@cursor.com>
…lict The hipSPARSE Math CI enforces clang-format v13, while the monorepo pre-commit uses v18. The two disagree on the alignment of the pre-existing Entry struct in amd_detail/utility.h, so any PR that modifies utility.h cannot satisfy both checks simultaneously. Relocate hipSpGEAMAlgToHCCSpGEAMAlg into the (flag-gated) SpGEAM wrapper and restore utility.h to develop-exact so it is no longer part of the PR diff. All other changed files are clean under both clang-format v13 and v18. Co-authored-by: Cursor <cursoragent@cursor.com>
jsandham
added a commit
that referenced
this pull request
Aug 10, 2026
## Motivation The documentation warnings: ``` /src/rocm-libraries/projects/hipsparse/docs/reference/generic.rst:346: WARNING: doxygenfunction: Cannot find function "hipsparseSpGEAM_createDescr" in doxygen xml output for project "hipsparse" from directory: doxygen/xml [docutils] /src/rocm-libraries/projects/hipsparse/docs/reference/generic.rst:351: WARNING: doxygenfunction: Cannot find function "hipsparseSpGEAM_destroyDescr" in doxygen xml output for project "hipsparse" from directory: doxygen/xml [docutils] /src/rocm-libraries/projects/hipsparse/docs/reference/generic.rst:356: WARNING: doxygenfunction: Cannot find function "hipsparseSpGEAM_bufferSize" in doxygen xml output for project "hipsparse" from directory: doxygen/xml [docutils] /src/rocm-libraries/projects/hipsparse/docs/reference/generic.rst:361: WARNING: doxygenfunction: Cannot find function "hipsparseSpGEAM_nnz" in doxygen xml output for project "hipsparse" from directory: doxygen/xml [docutils] /src/rocm-libraries/projects/hipsparse/docs/reference/generic.rst:366: WARNING: doxygenfunction: Cannot find function "hipsparseSpGEAM" in doxygen xml output for project "hipsparse" from directory: doxygen/xml [docutils] /src/rocm-libraries/projects/hipsparse/docs/reference/types.rst:118: WARNING: doxygentypedef: Cannot find typedef "hipsparseSpGEAMDescr_t" in doxygen xml output for project "hipsparse" from directory: doxygen/xml [docutils] /src/rocm-libraries/projects/hipsparse/docs/reference/types.rst:265: WARNING: doxygenenum: Cannot find enum "hipsparseSpGEAMAlg_t" in doxygen xml output for project "hipsparse" from directory: doxygen/xml [docutils] ``` started appearing after #9648 was merged. When warnings are treated like errors this causes the documentation builds to fail. This PR fixes these warnings.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add the generic
hipsparseSpGEAMAPI for sparse matrix-matrix addition,C = alpha * op(A) + beta * op(B), matching the cuSPARSE 13.3 genericSpGEAMAPI. This wraps the staged rocSPARSErocsparse_spgeamon the AMD/ROCm backend and passes through tocusparseSpGEAMon the NVIDIA/CUDA backend.hipsparseSpGEAM_createDescr,hipsparseSpGEAM_destroyDescr,hipsparseSpGEAM_bufferSize,hipsparseSpGEAM_nnz, andhipsparseSpGEAM, plus thehipsparseSpGEAMDescr_ttype and thehipsparseSpGEAMAlg_tenum (HIPSPARSE_SPGEAM_ALG1). All guarded byCUDART_VERSION >= 13030so headers still compile on CUDA toolkits without SpGEAM.HIP_R_32F/HIP_R_64F/HIP_C_32F/HIP_C_64F, and 32-/64-bit index types. Output C column indices are returned sorted.generic.rst,types.rst,api.rst) and the CHANGELOG are updated. A documentation example is provided.Testing
clients/tests(spgeam_csr,spgeam_csr_bad_arg) covering data types, index types, and edge cases.csrgeamreference.alpha/betazero edge cases are covered. Note: the sparsity pattern of C is the union of A and B and is independent of the scalar values on both backends (this matches cuSPARSE's documented behavior, where zero scalars are not a special case).HIPSPARSE_STATUS_NOT_SUPPORTEDin both backends (previously a transpose op was silently accepted on the ROCm backend).JIRA ID : AISPARSE-586
Made with Cursor
Made with Cursor