Skip to content

SpGEMM for HyKKT#366

Merged
shakedregev merged 27 commits intohykkt-devfrom
adham/hykkt-spgemm
Sep 2, 2025
Merged

SpGEMM for HyKKT#366
shakedregev merged 27 commits intohykkt-devfrom
adham/hykkt-spgemm

Conversation

@adhamsi
Copy link
Collaborator

@adhamsi adhamsi commented Aug 22, 2025

Description

This implements the Sparse General Matrix-Matrix multiplication (SpGEMM) module for the HyKKT solver. This provides the class SpGEMM.cpp with functions

  • addProductMatrices(matrix::Csr* A, matrix::Csr* B)
  • addSumMatrix(matrix::Csr* D)
  • addResultMatrix(matrix::Csr** E)
  • compute()

where compute will store the result of alpha * A * B + beta * D into E. The scalars are passed to the constructor.

Closes #308.

Proposed changes

The class is intended for efficient reuse. After an initial computation, the product and sum matrices can be reloaded with new values (and the same sparsity pattern) and compute() function called again. The addResultMatrix need only be called once.

On CPU, this is implemented with the CHOLMOD package in SuiteSparse. On CUDA, the cusparseSpGEMM and cusparseXcsrgeam functions for multiplication and addition respectively are used. On HIP, the rocsparse_spgemm function is called.

Checklist

Put an x in the boxes that apply. You can also fill these out after creating
the PR. If you're unsure about any of them, don't hesitate to ask. We're here
to help! This is simply a reminder of what we are going to look for before
merging your code.

  • All tests pass. Code tested on
    • CPU backend
    • CUDA backend
    • HIP backend
  • Code compiles cleanly with flags -Wall -Wpedantic -Wconversion -Wextra.
  • The new code follows Re::Solve style guidelines.
  • There are unit tests for the new code.
  • The new code is documented.
  • The feature branch is rebased with respect to the target branch.

Further comments

@adhamsi adhamsi force-pushed the adham/hykkt-spgemm branch from 65b842e to 36849b0 Compare August 26, 2025 16:52
@adhamsi adhamsi marked this pull request as ready for review August 29, 2025 16:12
@adhamsi adhamsi requested a review from shakedregev August 29, 2025 16:13
@adhamsi adhamsi self-assigned this Aug 29, 2025
Copy link
Collaborator

@shakedregev shakedregev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests should check if this can add a matrix with a differing sparsity structure (that is not a subset of the product).
I think this is doing $E=A*B+D$. The skipping of $C$ aside, we should put this equation in the documentation as it will be worth 1000 words.

@shakedregev
Copy link
Collaborator

shakedregev commented Aug 29, 2025

I would add a README to HyKKT explaining different rocm requirement.

@adhamsi
Copy link
Collaborator Author

adhamsi commented Sep 1, 2025

The tests should check if this can add a matrix with a differing sparsity structure (that is not a subset of the product). I think this is doing E = A ∗ B + D . The skipping of C aside, we should put this equation in the documentation as it will be worth 1000 words.

The test was modified so that the nonzero entries of D are no longer a subset of the nonzero entries of A * B. The test passes.

@shakedregev
Copy link
Collaborator

The tests pass and this looks good. There is a little bit more work on documentation, readme, and fixing these merge conflicts, which I can get to at some point.

@shakedregev shakedregev self-requested a review September 2, 2025 14:24
Copy link
Collaborator

@shakedregev shakedregev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added documentation and fixed the merge conflict (which was not a conflict at all, took both changes).

@shakedregev shakedregev merged commit 5588a30 into hykkt-dev Sep 2, 2025
6 checks passed
@shakedregev shakedregev deleted the adham/hykkt-spgemm branch September 2, 2025 14:48
shakedregev added a commit that referenced this pull request Nov 13, 2025
* main class and header

* begin cpu

* cpu implementation: no test yet

* handle csc vs csr and implement minimal test

* hip implemenation

* cuda implementation

* hip fix signature

* fix cmakelists

* minor fixes

* Apply pre-commmit fixes

* symbolic test

* reuse test

* fix for hip for reuse: broken

* Apply pre-commmit fixes

* compiler warning

* reuse test working

* reuse test cuda

* documentation

* Apply pre-commmit fixes

* fix build error

* Apply pre-commmit fixes

* rename add functions to load

* modify symbolic test for different case of D sparsity

* Apply pre-commmit fixes

* doc update

* straggling unsaved changes

* fixed typo

---------

Co-authored-by: adhamsi <adhamsi@users.noreply.github.com>
Co-authored-by: shakedregev <shakedvregev@gmail.com>
pelesh added a commit that referenced this pull request Nov 20, 2025
* Added the permutation class from hykkt, along with tests.

* Implement HyKKT Ruiz Scaling (#317)

* Implement the HyKKT Cholesky module (#350)

* SpGEMM for HyKKT (#366)

---------

Co-authored-by: superwhiskers <whiskerdev@protonmail.com>
Co-authored-by: Nicholson Koukpaizan <72402802+nkoukpaizan@users.noreply.github.com>
Co-authored-by: Adham Ibrahim <adham.ibrahim@princeton.edu>
Co-authored-by: kswirydo <kasia.swirydowicz@gmail.com>
Co-authored-by: shakedregev <shakedregev@users.noreply.github.com>
Co-authored-by: Slaven Peles <peless@ornl.gov>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants