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.

Sign up for GitHub

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

Jump to bottom

CUDA acceleration always enabled regardless of cuda feature enabled #259

Closed
samuelburnham opened this issue Jan 8, 2024 · 3 comments · Fixed by lurk-lab/grumpkin-msm#9
Closed

CUDA acceleration always enabled regardless of cuda feature enabled #259

samuelburnham opened this issue Jan 8, 2024 · 3 comments · Fixed by lurk-lab/grumpkin-msm#9
Assignees
@winston-h-zhang
Labels
bug Something isn't working P-Performance

Comments

@samuelburnham
Copy link
Contributor

There have been multiple reports of consistent GPU utilization shown with nvtop on Nvidia machines, regardless of if the cuda/opencl features are enabled. However, there should be no CUDA usage if the feature isn't enabled.

According to @winston-h-zhang this may be due to the following issue in https://github.com/lurk-lab/grumpkin-msm and https://github.com/supranational/pasta-msm:

build.rs will always enable cuda if it detects a valid nvcc
@samuelburnham samuelburnham added bug Something isn't working P-Performance labels Jan 8, 2024
@winston-h-zhang winston-h-zhang self-assigned this Jan 9, 2024
@winston-h-zhang winston-h-zhang mentioned this issue Jan 9, 2024
Merged
huitseeker referenced this issue in huitseeker/arecibo Jan 12, 2024
@huitseeker
fix: Integrate CUDA feature with grumpkin_msm dependency
054f473 
- Modified grumpkin-msm dependency to be optional for x86_64/aarch64 targets and added it to CUDA dependencies in `Cargo.toml`.
- Inserted `cuda` feature guard for `vartime_multiscalar_mul` function in `traits.rs` file to only allow execution when `cuda` feature and specific architecture are enabled.
- Adjusted targeting condition for grumpkin_msm algorithm use in `pasta.rs`. The algorithm is now dependent on the 'cuda' feature and x86_64 or aarch64 architecture.
- Updated conditional compilation in `bn256_grumpkin.rs` to integrate CUDA feature for both bn256 and grumpkin.

Fixes https://github.com/lurk-lab/lurk-rs/issues/1030
@huitseeker huitseeker mentioned this issue Jan 12, 2024
Closed
@huitseeker
Copy link
Contributor

huitseeker commented Jan 12, 2024 •
edited
Loading

My tests on CPU comparing dev at 2066ae4 with #257.
results (1st set is dev/, 2nd set is the PR, machine is an M1 Mac):
https://gist.github.com/huitseeker/0a5504320bd179f07310de131f618c72

=> grumpkin-msm's CPU msm is better (for now) than halo2curves

@huitseeker huitseeker transferred this issue from lurk-lab/lurk-beta Jan 12, 2024
@huitseeker
Copy link
Contributor

Sam's results:
https://gist.github.com/huitseeker/b1b567f2aa4396796cd9bd9ea564a1e4
w/o the non-asm run:
https://gist.github.com/huitseeker/0a5504320bd179f07310de131f618c72

huitseeker pushed a commit to huitseeker/arecibo that referenced this issue Jan 14, 2024
@srinathsetty
Simplify Group trait (lurk-lang#259)
8cb4bec 
* simplify Group trait

* fix clippy

* further simplifications

* fix Copy
@huitseeker
Copy link
Contributor

Conclusion: we should make grumpkin-msm:
1/ always compile the CPU method, as it does now,
2/ only attempt to compile and run the GPU method when the cuda feature is enabled
3/ emit an indiscutably spectacular message when it's compiled with the cuda feature and does not find the CPU,

See PR at lurk-lab/grumpkin-msm#9

@winston-h-zhang winston-h-zhang mentioned this issue Jan 23, 2024
Closed
gabriel-barrett pushed a commit to gabriel-barrett/arecibo that referenced this issue Feb 9, 2024
@huitseeker @srinathsetty
Nova forward ports (lurk-lang#126)
8adb3ce 
* Simplify Group trait (lurk-lang#259)

* simplify Group trait

* fix clippy

* further simplifications

* fix Copy

* Cut duplicate code (lurk-lang#261)

* cut duplicate code

* bound --> bind for clarity

* expose asm feature

* test: check test_pp_digest with asm feature in CI

* expose size of the base field (lurk-lang#262)

* fix: add mention of halo2curves issue explaining the problem

---------

Co-authored-by: Srinath Setty <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@winston-h-zhang winston-h-zhang

Labels
bug Something isn't working P-Performance
Projects
None yet
Milestone
No milestone
3 participants
@huitseeker @samuelburnham @winston-h-zhang