-
Notifications
You must be signed in to change notification settings - Fork 3
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
fix: Add a working cuda
feature
#9
Conversation
@@ -0,0 +1,33 @@ | |||
# Runs the test suite on a self-hosted GPU machine with CUDA enabled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: we're only building here, not testing. Is the latter intended for later or not a priority?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- do we really need to push the semantics of the cuda feature inside grumpkin-msm or can we simply not import grumpkin-msm in dependents if that feature is not activated?
- can we make grumpkin-msm print an indiscutably spectacular message if it's loaded and does not manage to resolve to a GPU-using MSM? I really want the fallback to the CPU to be extremely noticeable, 100% of the time, every time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To reproduce the comment from lurk-lang/arecibo#259, this is going in a good direction given the fresh benchmarks results, but:
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,
c45f47f
to
a36a643
Compare
a36a643
to
498c207
Compare
This is the error message:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent, thanks a lot!
Resolves lurk-lang/arecibo#259 downstream. This PR adds an actual
cuda
feature that can be toggledgrumpkin-msm
, and thus all of its dependents. We also add GPU CI checks.