Migrate pecos-random to rapidrand with stream-identical seeding#356
Merged
Conversation
…preserving seeded streams
# Conflicts: # Cargo.lock # Cargo.toml
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.
rapidhash 4.5 deprecated its
rngmodule ("use therapidrandcrate instead"), which is what tripped PR #355's lint gates and led to the temporary<4.5hold. This is the proper migration.The determinism question, settled empirically
Seeded reproducibility is load-bearing (users record seeds to reproduce runs), so the swap was gated on stream identity, not assumed:
rapidrand's mixing function and secrets are byte-identical to rapidhash 4.4'srapidrng_fast(verified against both crates' sources).RapidRng::new(seed)stored the seed raw; rapidrand'sseed_from_u64pre-mixes (different stream), whileSeedableRng::from_seed(seed.to_le_bytes())is raw. Verified with a side-by-side harness over seeds {0, 1, 42, 0xDEADBEEF, u64::MAX}:from_seedis bit-identical to the old stream;seed_from_u64is not.Changes
pecos_random::rapid_rng::RapidRngshim wrappingrapidrand::RapidRngwith the legacy semantics (new(u64)= rawfrom_seed;next()), documented in one place — all existing call sites inrng.rs/scalar_rng.rsunchanged.tests/seed_stream_stability.rs: locks the first 8 outputs ofParallelRapidRngandPecosScalarRngat seed 42 to values captured from the pre-migration implementation and verified passing before the swap — any future accidental reproducibility break fails loudly.statistical_quality.rsand the RNG benchmarks migrated the same way (raw from_seed, so bench comparability holds).rapidhashremoved from the workspace entirely (no other consumers);rapidrand 0.1.2added.Verification
Golden stream test passes on both sides of the migration (the actual oracle); full pecos-random suite green;
cargo check --workspaceclean; clippy clean on pecos-random (all targets) + benchmarks; fmt clean.Merge-order note: #355 holds
rapidhash = "4, <4.5"; this PR deletes the dependency. Whichever merges second takes a trivialCargo.toml/Cargo.lockconflict — happy to rebase this one after #355 lands.