Skip to content

Fix uint32 multiplication in PCG next and peel functions - #28

Open
Janiczek wants to merge 1 commit into
elm:masterfrom
Janiczek:patch-1
Open

Fix uint32 multiplication in PCG next and peel functions#28
Janiczek wants to merge 1 commit into
elm:masterfrom
Janiczek:patch-1

Conversation

@Janiczek

@Janiczek Janiczek commented Aug 1, 2026

Copy link
Copy Markdown

JavaScript multiplication loses precision on large numbers (when the result gets above Number.MAX_SAFE_INTEGER = 2^53 - 1). In elm/random this happens in next and peel.

The JavaScript solution is to use Math.imul() (MDN), which in Elm we need to emulate by splitting the numbers in two 16bit halves and carefully manipulating them, unless a new stdlib primitive is added which calls Math.imul.

The consequence for elm/random is that the numbers returned from next and the state advanced by peel slowly diverges from the equivalent C implementation.

For initialSeed = 123456789 -> state = 1102901571, incr = 1013904223, we get:

Elm next C next Difference
2615485039 2615483789 1250
848452106 848452205 -99
1351529346 1351529358 -12
999259678 999259672 6
51789132 51789135 -3
2069463533 2069463525 8

It's unclear to me what the severity of this deviance from PCG is: we'd need to run Dieharder or some other randomness statistical test suite on the Elm numbers. I presume since precision is lost in the multiplication (the upper 11 bits or so), the mixing stops being as good over time and some statistical properties get lost, but I haven't run the tests (yet).

This PR makes Elm return the same number sequences as C would.

@Janiczek

Janiczek commented Aug 1, 2026

Copy link
Copy Markdown
Author

Update: here are the Dieharder results for the current (1.0.0) buggy version and for the proposed fix:

pcgelmbuggy_c_result.txt
pcgelmfixed_c_result.txt

Summary

Algorithm Passed Weak Failed
Current, buggy 97 3 14
Proposed, fixed 111 3 0

So even taking these summary results blindly, it seems fixing next and peek would make the random number generator behave better in these statistical tests, perhaps at performance expense of the emulated imul32 (but the performance hit should be lower if the real Math.imul function is exposed as a Bitwise primitive).

A related question is, if this fix gets accepted in some form (changing the random numbers generated for a given seed), should elm/random be artifically bumped from 1.0.1 to 2.0.0 to signal that something significant happened?

@Janiczek

Janiczek commented Aug 2, 2026

Copy link
Copy Markdown
Author

Some more progress on this: I ran Dieharder with -k 2 -Y 1 which made it resolve those WEAK results to either PASSING or FAILING. The fixed version of elm/random now passes all 114 tests.

But then I ran the TestU01 BigCrush tests, and elm/random fails 12 (v1.0.0) or 9 (this PR) statistical tests there! This is surprising because I'd expect PCG to pass everything you throw at it. This suggests that maybe our implementation of PCG isn't faithful enough to the original C. Maybe our choices of constants are wrong, or the algorithm is slightly off, or something.

I wrote a blogpost about (among other things) this exploration: https://martin.janiczek.cz/2026/08/02/improving-elm-prng.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant