Make CUDA the default compute platform - #1576
Conversation
|
I know we like to use |
I might be missing something, we're storing the string as-is, unless you mean something else? |
|
I see what you are saying, we use |
That's the behaviour we've always had - if you want to change this, I would suggest we do this in a separate PR. I'm only scoping this as "changing the default from None to cuda". |
|
|
||
| @field_validator('compute_platform') | ||
| def supported_sampler(cls, v): | ||
| supported = ['cpu', 'opencl', 'cuda'] |
There was a problem hiding this comment.
Just adding a link here so if we ever decide we need HIP support, I can find this easily
mikemhenry
left a comment
There was a problem hiding this comment.
LGTM! Really just wondering if it makes sense to set s.protocol_repeats = 1 in the testing fixtures instead of in most of the tests that use the fixture
There was a problem hiding this comment.
I am not sure if any tests here actually need more than one repeat
Thanks, I will manually check and update - apologies in advance, for my own sanity I might not merge your suggestions from the github interface (so I can keep track of what has and hasn't been updated). |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1576 +/- ##
==========================================
- Coverage 95.45% 93.42% -2.03%
==========================================
Files 174 174
Lines 14552 14534 -18
==========================================
- Hits 13891 13579 -312
- Misses 661 955 +294
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
That is fine! I haven't been able to get the "add suggestion to batch" to work with the new PR UI anyway so it would have been a separate commit for each one. I mostly did it so I could keep track of how many there were set to one versus how many were set to the default. |
|
Ok that's all of the protocol repeats dealt with @mikemhenry ! |
|
I'm happy with it! |
|
No API break detected ✅ |
|
Failure is unrelated to this PR - merging so we can fix other things elsewhere. |
TODO:
Checklist
newsentryDevelopers certificate of origin