Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
ENH: Add PCG64DXSM BitGenerator #18906
ENH: Add PCG64DXSM BitGenerator #18906
Changes from 6 commits
1cb1f67
771ce9d
8194336
32ebda5
781254a
5d380b9
dc8cd57
8e4d3e1
a1142e3
aa6733b
d82ef18
d97c634
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
"pool is less likely"?
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.
No. It is more likely that you will observe a collision in the
SeedSequence
pool (creating completely identical seeds) than to see one of the poorly-correlated pairs of distinct streams.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.
Changing the verb to "would be" would help (and be more consistent with the other verbiage we have on the subject).
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.
Does the
SeedSequence
affect both increment and state, or just the state?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.
Both increment and state. During the mega-thread, we decided that was the best design, especially once we got strong seeding with
SeedSequence
.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.
Does this make the seed sequences differ between bit-generators or is the increment derived from the state?
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.
Let me back up a bit to where I suspect the disconnect is. We'll see if I guessed right.
SeedSequence
takes an arbitrary sequence of (arbitrary-sized) integers and hashes them down into an internal pool of bits. The default pool size is 128 bits. TheBitGenerator
then asks theSeedSequence
for however many bits it needs to initialize itself. ForMT19937
, that's a plump 624uint32
s. For thePCG64
s, that's the relatively-svelte 128-bit state and 127-bit increment (added together and rounded up to an even 256 bits total). TheSeedSequence
uses what amounts to a slow, carefully constructed PRNG to generate however many bits are requested. It's not just copying its internal pool of 128 bits into theBitGenerator
state.The claim I am making here is that the odds that two distinct
SeedSequence
s generate a pair of distinct, badPCG64DXSM
instances is less than the odds that two distinct user inputs hashed down to the same 128-bit internal pool. And the odds of that 128-bit collision are ignorably tiny.