Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SpMVM acceleration on the CPU #368

Closed
huitseeker opened this issue Mar 18, 2024 · 2 comments
Closed

SpMVM acceleration on the CPU #368

huitseeker opened this issue Mar 18, 2024 · 2 comments
Assignees
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@huitseeker
Copy link
Contributor

What?

Nova requires committing to a cross-term which involves a sparse matrix-vector multiplication, which is currently a folding bottleneck.

Context

We changed our matrix representation from the original COO (still used in the Spartan2 repo mentioned below) to CSR and parallelized the matrix-vector multiplication in #38.
Ref on this data representation formats: https://en.wikipedia.org/wiki/Sparse_matrix#Coordinate_list_(COO)

We have an issue open for accelerating this on the GPU at #75

This issue

We'd like to cherry-pick the improvements from https://github.com/a16z/Spartan2/pulls?q=is%3Apr+matrix+is%3Aclosed to see if e.g. a specialization to the one-scalar would help the SpMVM on the CPU

@huitseeker huitseeker added good first issue Good for newcomers help wanted Extra attention is needed labels Mar 18, 2024
@winston-h-zhang winston-h-zhang self-assigned this Mar 18, 2024
@winston-h-zhang
Copy link
Contributor

Data from #370 suggests that there are not much improvements using a16z's Spartan2 implementation, at least at first glance. Some of the verify tests seem to be performing better, but I'm not sure why. Otherwise, it seems that using a chunked multiply and specializing Scalar::ONE has minimal impact on performance. I guess, since we already have a parallelized implementation, the most low-hanging fruit has already been addressed

@huitseeker
Copy link
Contributor Author

I agree, thanks for ruling that out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants