Skip to content
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

RandomSourceBase.NextDoubleNonZero() can produce zero #5

Closed
colgreen opened this issue Jul 3, 2020 · 2 comments
Closed

RandomSourceBase.NextDoubleNonZero() can produce zero #5

colgreen opened this issue Jul 3, 2020 · 2 comments

Comments

@colgreen
Copy link
Owner

colgreen commented Jul 3, 2020

This:

return ((NextULongInner() >> 11) & 0x1f_ffff_ffff_fffe) * INCR_DOUBLE;

Should be:

return (((NextULongInner() >> 11) & 0x1f_ffff_ffff_fffe)+1) * INCR_DOUBLE;

Otherwise a zero will be returned when NextULongInner() returns zero, which is approximately every 2^64 calls (approx. 1.8*10^19) if the underlying PRNG is capable of generating zero, which all of the redzen PRNGs are. This makes the error an exceedingly rare event, but it is possible and thus a definite bug. E.g. if this method were called a billion times a second, then we can expect to see a zero approximately once every 31 years!

colgreen added a commit that referenced this issue Jul 5, 2020
- Doubled sampling resolution of NextDoubleNonZero().
- Added IRandomSource.NextFloatNonZero().
- Reordered methods in IRandomSource to a slightly more logical order
- Removed [MethodImpl(MethodImplOptions.AggressiveInlining)] in a few places, as the JIT compiler will inline where appropriate without this hint.
- Added RandomSourceBaseTests; this performs bounds checks on most of the IRandomSource methods.
colgreen added a commit that referenced this issue Jul 5, 2020
Fix: NextDoubleNonZero() could return zero (#5)
@colgreen
Copy link
Owner Author

colgreen commented Jul 5, 2020

Fixed in the 9.x maintenance branch in release 9.1.1.

A fix for the master branch is pending.

@colgreen
Copy link
Owner Author

colgreen commented Aug 2, 2020

Fixed in master branch, and v11.0.0 release.

@colgreen colgreen closed this as completed Aug 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant