-
Notifications
You must be signed in to change notification settings - Fork 207
Fix thrust::uniform_int_distribution last 12-bits always being 0 #4393
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
base: main
Are you sure you want to change the base?
Conversation
2a523e3
to
9768035
Compare
@gonidelis Thanks for this!
This will be the case where it produces a small value that can be represented accurately in the mantissa, notice there is an extra leading zero (omitted by the print) for that case |
|
||
// Add random lower bits to fix last 12-bits always being 0. | ||
result_type lower_bits = static_cast<result_type>(urng()) & 0xFFF; // Get 12 **uniformly** random bits | ||
return base | lower_bits; // Combine them |
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.
Is there a particular reason we use the real_dist, instead of using an algorithm that can directly generate integers.
Also won't this break for int distributions < 12 bits
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.
Is there a particular reason we use the real_dist, instead of using an algorithm that can directly generate integers.
It's legacy, unfortunately I don't know.
Also won't this break for int distributions < 12 bits
yes, good catch. fixing it rn.
🟨 CI finished in 4h 52m: Pass: 88%/101 | Total: 2d 00h | Avg: 29m 00s | Max: 1h 30m | Hits: 78%/121761
|
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
CUB | |
+/- | Thrust |
CUDA Experimental | |
stdpar | |
python | |
CCCL C Parallel Library | |
Catch2Helper |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
+/- | CUB |
+/- | Thrust |
CUDA Experimental | |
+/- | stdpar |
+/- | python |
+/- | CCCL C Parallel Library |
+/- | Catch2Helper |
🏃 Runner counts (total jobs: 101)
# | Runner |
---|---|
72 | linux-amd64-cpu16 |
9 | windows-amd64-cpu16 |
6 | linux-arm64-cpu16 |
6 | linux-amd64-gpu-rtxa6000-latest-1 |
3 | linux-amd64-gpu-h100-latest-1 |
3 | linux-amd64-gpu-rtx4090-latest-1 |
2 | linux-amd64-gpu-rtx2080-latest-1 |
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
fixes #758
Due to
thrust::uniform_int_distribution
relying onuniform_real_distribution
the last 12-bits, as suggested in the tracking issue, are always0
. When I run the code from #758 (but with a fulldefault_random_engine
) I get output that looks like:I am still not entirely sure why 3rd to last (even the 2nd in some rare cases) digit sometimes is not
0
.If a full engine is used the trailing zeros will (or at least should) be 3 and this PR fixes that by bit-manipulation. After calculating the base as before, we produce a new
12-bit
uniformly random number that we add/append to the base. The result is a new uniformly random number.Note
User needs to take care to use a proper engine. The code from the issue is using a
48-bit
engine. That's why there are 4 trailing0
s, and not 3 as expected when I run the source from the issue withranlux48
. Detailed explanation in the comment under the issue.