-
Notifications
You must be signed in to change notification settings - Fork 135
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
Drop thrusts diagnostic suppression warnings #2392
Drop thrusts diagnostic suppression warnings #2392
Conversation
@gevtushenko this might be a tad controversial, but I am of the opinion that the thrust warning suppression macros should be considered as implementation details and not user facing. A user can always silence warnings on their own |
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.
I love the refactoring, but removing the macros is technically a breaking change :S
It technically is but iin my opinion warning suppression is an implementation detail that should have never been public and I am willing break that |
0d12319
to
bba5d52
Compare
🟩 CI finished in 11h 00m: Pass: 100%/259 | Total: 6d 22h | Avg: 38m 31s | Max: 1h 19m | Hits: 34%/24441
|
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
CUB | |
+/- | Thrust |
CUDA Experimental | |
pycuda | |
CUDA C Core Library |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
+/- | CUB |
+/- | Thrust |
CUDA Experimental | |
+/- | pycuda |
+/- | CUDA C Core Library |
🏃 Runner counts (total jobs: 259)
# | Runner |
---|---|
186 | linux-amd64-cpu16 |
42 | linux-amd64-gpu-v100-latest-1 |
16 | linux-arm64-cpu16 |
15 | windows-amd64-cpu16 |
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.
@gevtushenko this might be a tad controversial, but I am of the opinion that the thrust warning suppression macros should be considered as implementation details and not user facing.
I agree, these macro should've been an implementation detail. If we were to keep them part of public API, we'd have to add compilation tests and documentation.
We have global suppression warnings for CCCL, so use them
bba5d52
to
bd04ca6
Compare
🟩 CI finished in 1h 44m: Pass: 100%/259 | Total: 6d 16h | Avg: 37m 04s | Max: 1h 12m | Hits: 34%/24432
|
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
CUB | |
+/- | Thrust |
CUDA Experimental | |
pycuda | |
CUDA C Core Library |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
+/- | CUB |
+/- | Thrust |
CUDA Experimental | |
+/- | pycuda |
+/- | CUDA C Core Library |
🏃 Runner counts (total jobs: 259)
# | Runner |
---|---|
186 | linux-amd64-cpu16 |
42 | linux-amd64-gpu-v100-latest-1 |
16 | linux-arm64-cpu16 |
15 | windows-amd64-cpu16 |
We have global suppression warnings for CCCL, so use them
This is helpful for #2386