Fix Issue 10550 - Xorshift32 and Xorshift160 do not generate uniformly-distributed random numbers #1403
Conversation
The original typo derives directly from George Marsaglia's paper introducing the Xorshift RNGs. The original checking values, derived from that code, thus also reflect the typo. A corrected version of the reference code is available from https://github.com/WebDrake/xorshift/ and has been used to generate the updated checking values.
The original typo derives directly from George Marsaglia's paper introducing the Xorshift RNGs. The original checking values, derived from that code, thus also reflect the typo. A corrected version of the reference code is available from https://github.com/WebDrake/xorshift/ and has been used to generate the updated checking values.
ghost
assigned
Fix Issue 10550 - Xorshift32 and Xorshift160 do not generate uniformly-distributed random numbers
|
Good catch! |
| { | ||
| static assert(false, "Xorshift supports only 32, 64, 96, 128, 160 or 192 bit versions. " | ||
| ~ to!string(bits) ~ " is not supported."); | ||
| } |
There was a problem hiding this comment.
There was already a static assert above that checked this:
static assert(bits == 32 || bits == 64 || bits == 96 || bits == 128 || bits == 160 || bits == 192,
"Supporting bits are 32, 64, 96, 128, 160 and 192. " ~ to!string(bits) ~ " is not supported.");While an explicit check for 192 is always clearer and makes the code more robust, the only diagnostic here worth throwing would be: "Phobos Error". Because if we hit that line, that's what it is.
There was a problem hiding this comment.
There was already a static assert above that checked this
Yes. But we can add other bits version to this engine (e.g. 256 bits),
so I think each check is robust than old implementation.
There was a problem hiding this comment.
Yea, I deliberately added the double check. I thought it was important to be absolutely explicit about each possible value for the number of bits, and have a double-locked guarantee.
On that note, I just realized I missed the static if/else in popFront(). I'll push a separate fix for that.
Corrects a number of apparent typos in George Marsaglia's original paper describing Xorshift RNGs, which result in severe deviations from uniformity in the random variates generated by Xorshift32 and Xorshift160.
The uniformity of the random variates produced has been verified casually but not yet systematically (e.g. with a high-quality random number test suite such as dieharder or TestU01).
The updated checking values have been generated from a corrected copy of Marsiglia's reference code, available from https://github.com/WebDrake/xorshift