Skip to content

Commit 7c85104

Browse files
committed
random: Add clarifying comments to the implementation of CombinedLCG
The implementation is needlessly obfuscated. It's not immediately clear that MODMULT is a simple modular multiplication, despite its name. Specifically it's not clear which of the parameters is the second factor. Furthermore the stated period is off-by-one: A value of `0` is part of its own chain, thus it may not be included in the period of the underlying generators.
1 parent cf31332 commit 7c85104

File tree

1 file changed

+7
-2
lines changed

1 file changed

+7
-2
lines changed

ext/random/engine_combinedlcg.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,11 @@
2727
/*
2828
* combinedLCG() returns a pseudo random number in the range of (0, 1).
2929
* The function combines two CGs with periods of
30-
* 2^31 - 85 and 2^31 - 249. The period of this function
31-
* is equal to the product of both primes.
30+
* 2^31 - 85 - 1 and 2^31 - 249 - 1. The period of this function
31+
* is equal to the product of the two underlying periods, divided
32+
* by factors shared by the underlying periods, i.e. 2.3 * 10^18.
33+
*
34+
* see: https://library.sciencemadness.org/lanl1_a/lib-www/numerica/f7-1.pdf
3235
*/
3336
#define MODMULT(a, b, c, m, s) q = s / a; s = b * (s - a * q) - c * q; if (s < 0) s += m
3437

@@ -43,7 +46,9 @@ static php_random_result generate(void *state)
4346
php_random_status_state_combinedlcg *s = state;
4447
int32_t q, z;
4548

49+
/* s->state[0] = (s->state[0] * 40014) % 2147483563; */
4650
MODMULT(53668, 40014, 12211, 2147483563L, s->state[0]);
51+
/* s->state[1] = (s->state[1] * 40692) % 2147483399; */
4752
MODMULT(52774, 40692, 3791, 2147483399L, s->state[1]);
4853

4954
z = s->state[0] - s->state[1];

0 commit comments

Comments
 (0)