<random>: Implement modified ziggurat algorithm for normal_distribution#6104
Open
statementreply wants to merge 18 commits into
Open
<random>: Implement modified ziggurat algorithm for normal_distribution#6104statementreply wants to merge 18 commits into
<random>: Implement modified ziggurat algorithm for normal_distribution#6104statementreply wants to merge 18 commits into
Conversation
StephanTLavavej
requested changes
Feb 23, 2026
StephanTLavavej
requested changes
Feb 23, 2026
StephanTLavavej
requested changes
Feb 23, 2026
|
|
||
| _STD_BEGIN | ||
|
|
||
| template <class _Ty, class _Uty, bool _Signed, int _Lx> |
Contributor
There was a problem hiding this comment.
Can we remove _Signed is it's always true?
Suggested change
| template <class _Ty, class _Uty, bool _Signed, int _Lx> | |
| template <class _Ty, class _Uty, int _Lx> |
Contributor
Author
There was a problem hiding this comment.
It's reserved for exponential_distribution, where _Signed would be false.
| static_assert(_Lx <= 254, "invalid table size"); | ||
|
|
||
| using _Uint_type = _Uty; | ||
| using _Xtype = conditional_t<_Signed, make_signed_t<_Uty>, _Uty>; |
Contributor
There was a problem hiding this comment.
Suggested change
| using _Xtype = conditional_t<_Signed, make_signed_t<_Uty>, _Uty>; | |
| using _Xtype = make_signed_t<_Uty>; |
Also, would it be better to rename it to _Sint_type?
| using _Xtype = conditional_t<_Signed, make_signed_t<_Uty>, _Uty>; | ||
|
|
||
| static constexpr int _Layer_num = _Lx; | ||
| static constexpr _Ty _Width_scale = (_Signed ? _Ty{1} : _Ty{2}) * _Ty{(numeric_limits<_Uty>::max)() / 2u + 1u}; |
Contributor
There was a problem hiding this comment.
Let's avoid numeric_limits.
Suggested change
| static constexpr _Ty _Width_scale = (_Signed ? _Ty{1} : _Ty{2}) * _Ty{(numeric_limits<_Uty>::max)() / 2u + 1u}; | |
| static constexpr _Ty _Width_scale = _Ty{static_cast<_Uty>(-1) / 2u + 1u}; |
Co-authored-by: A. Jiang <de34@live.cn>
| const _Fty _Ydiff = _Yy1 - _Yy0; | ||
|
|
||
| for (;;) { | ||
| const _Fty _Dx = _Xdiff * _STD _Nrand_impl<_Fty>(_Eng); |
Contributor
There was a problem hiding this comment.
The upper bits of _Xbits could be used on the first iteration. Also for the tail, below.
mohamed1rashad
approved these changes
Mar 10, 2026
After using 1 bit for the sign and 24 bits for the magnitude of the random `float` from the 32-bit random value, we have only 7 unused bits.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR implements modified ziggurat algorithm for
normal_distribution, based on McFarland, C. D. (2015). A modified ziggurat algorithm for generating exponentially and normally distributed pseudorandom numbers.The main differences between this PR and the paper:
Benchmark results for x64 on Intel Core i5-13600KF:
BM_Generator<std::mt19937_64>BM_Generator<b_r::mt19937_64>BM_Distribution<std::mt19937_64, std_old::normal_distribution<double>>BM_Distribution<std::mt19937_64, std_new::normal_distribution<double>>BM_Distribution<std::mt19937_64, boost_r::normal_distribution<double>>BM_Distribution<b_r::mt19937_64, boost_r::normal_distribution<double>>BM_Generator<std::mt19937>BM_Generator<b_r::mt19937>BM_Distribution<std::mt19937, std_old::normal_distribution<float>>BM_Distribution<std::mt19937, std_new::normal_distribution<float>>BM_Distribution<std::mt19937, boost_r::normal_distribution<float>>BM_Distribution<b_r::mt19937, boost_r::normal_distribution<float>>Removed
tests\GH_004618_normal_distribution_avoids_resetswhich became moot.Fixes #1003.