Repository navigation
tests: Pin random number generator outputs in tests - #7634
Merged
Merged
Conversation
Adds tests that pin the exact output of the library drand48-family PRNG (G_lrand48, G_mrand48, G_drand48) for fixed seeds and check that re-seeding restarts the sequence. In addition to reference values in the tests, a local test against POSIX implementation showed the same values. I generated the tests and did the local testing with Claude (Fable) which came up with the test against POSIX implementation, but I'm leaving it as a note only because correspondence to POSIX is not a feature of our implementation. This is groundwork for OSGeo#6480 (thread-safe random number generation) and complements OSGeo#7633 (random numbers in r.mapcalc).
github-actions
Bot
added
Python
echoix
approved these changes
Jul 4, 2026
ninsbl
pushed a commit
to ninsbl/grass
that referenced
this pull request
Jul 7, 2026
wenzeslaus
added a commit
to wenzeslaus/grass
that referenced
this pull request
Aug 18, 2026
Replace the pthread mutex approach with a lock-free compare-and-swap on the whole 48-bit generator state kept in a single C11 atomic integer. One successful compare-and-swap is exactly one step of the standard drand48 linear congruential generator, so the sequence of generated values for a given seed is identical to a single-threaded run and to the previous limb-based implementation. Compared to the mutex: the feature is gated by the compiler (__STDC_NO_ATOMICS__), not by --with-pthread, so it is active in every supported build, including the OpenMP-without-pthread configurations (macOS CI, Docker images) where the race reported in OSGeo#5742 actually occurs; no build system changes or extra link dependencies are needed (the Makefile and CMake changes are dropped, which also avoids defining HAVE_PTHREAD from the Win32-threads CMake package on MSVC); and there is no lock contention (in a microbenchmark the mutex made threaded runs slower than single-threaded at every thread count, while the atomic version is about twice as fast as the mutex when several threads generate numbers at the same time). A single shared generator still cannot scale with the number of threads; per-thread state remains future work. Where C11 atomics are unavailable, the code falls back to the previous plain implementation, which is not thread-safe (unchanged behavior). Output compatibility is guarded by the PRNG regression tests merged in OSGeo#7633 and OSGeo#7634 (lib/gis/tests/lib_gis_lrand48_test.py and raster/r.mapcalc/tests/r_mapcalc_rand_seed_test.py) and was additionally verified against the previous implementation for multiple seeds. The 64-bit reformulation of the generator and its verification were AI-assisted (Claude).
wenzeslaus
added a commit
to wenzeslaus/grass
that referenced
this pull request
Aug 18, 2026
Replace the pthread mutex approach with a lock-free compare-and-swap on the whole 48-bit generator state kept in a single C11 atomic integer. One successful compare-and-swap is exactly one step of the standard drand48 linear congruential generator, so the sequence of generated values for a given seed is identical to a single-threaded run and to the previous limb-based implementation. Compared to the mutex: the feature is gated by the compiler (__STDC_NO_ATOMICS__), not by --with-pthread, so it is active in every supported build, including the OpenMP-without-pthread configurations (macOS CI, Docker images) where the race reported in OSGeo#5742 actually occurs; no build system changes or extra link dependencies are needed (the Makefile and CMake changes are dropped, which also avoids defining HAVE_PTHREAD from the Win32-threads CMake package on MSVC); and there is no lock contention (in a microbenchmark the mutex made threaded runs slower than single-threaded at every thread count, while the atomic version is about twice as fast as the mutex when several threads generate numbers at the same time). A single shared generator still cannot scale with the number of threads; per-thread state remains future work. Where C11 atomics are unavailable, the code falls back to the previous plain implementation, which is not thread-safe (unchanged behavior). Output compatibility is guarded by the PRNG regression tests merged in OSGeo#7633 and OSGeo#7634 (lib/gis/tests/lib_gis_lrand48_test.py and raster/r.mapcalc/tests/r_mapcalc_rand_seed_test.py) and was additionally verified against the previous implementation for multiple seeds. The 64-bit reformulation of the generator and its verification were AI-assisted (Claude).
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.
Adds tests that pin the exact output of the library drand48-family PRNG (G_lrand48, G_mrand48, G_drand48) for fixed seeds and check that re-seeding restarts the sequence.
In addition to reference values in the tests, a local test against POSIX implementation showed the same values.
I generated the tests and did the local testing with Claude (Fable) which came up with the test against POSIX implementation, but I'm leaving it as a note only because correspondence to POSIX is not a feature of our implementation.
This is groundwork for #6480 (thread-safe random number generation) and complements #7633 (random numbers in r.mapcalc).