Implementations of SFC32/SFC64 chaotic RNGs#65
Implementations of SFC32/SFC64 chaotic RNGs#65tertu-m wants to merge 10 commits intorust-random:masterfrom
Conversation
dhardy
left a comment
There was a problem hiding this comment.
At a glance, this seems like something we could add. But should we?
- I'd like some justification of why this is useful to people.
- Would you be willing to maintain this in case of future bug reports / requests?
rand_sfc/src/lib.rs
Outdated
| @@ -0,0 +1,39 @@ | |||
| // Copyright 2025 XXX. | |||
There was a problem hiding this comment.
I'll update that to mention the project now.
|
I would be willing to maintain this in the future. My reasons for suggesting they are included that they are simple, fast generators (in my experience and based on what I could find on the internet they are slightly faster than Xoshiro and faster than PCG), they are high quality, they are reasonably well known as far as I am aware, and they are structurally different from the existing generators present in rand. Additionally, in principle, stream support could be added as I mentioned in the comments, and this has been done by some people, but I don't think that functionality is very well studied. It is known that correlations exist early in two streams with different increments if the initial values provided for EDIT: I guess you knew all that already; I read the comments later where that was mentioned. Anyway I guess it had been talked about in the past and the thought was maybe one more choice would be nice? This implementation doesn't have the counter panic issue, anyway. |
dhardy
left a comment
There was a problem hiding this comment.
I'll accept that argument.
I started a review, but didn't get as far as the actual RNG code yet.
rand_sfc/Cargo.toml
Outdated
| [features] | ||
| serde1 = ["serde"] |
There was a problem hiding this comment.
Please use serde = ["dep:serde"]
rand_sfc/src/sfc32.rs
Outdated
| // PracRand uses different mixing step counts for different types of seeds. | ||
| // Here, just use one of the larger values always. | ||
| const SEED_MIXING_STEPS: u32 = 16; |
There was a problem hiding this comment.
PracRand uses 8 for seed_fast(u64), 12 for seed(u64) or 15 for seed(u32, u32, u32). Perhaps we should use the latter?
| fn reference() { | ||
| // These values were produced with the reference implementation: | ||
| // https://pracrand.sourceforge.net/ | ||
| let mut rng = Sfc32::from_seed([0, 0, 0, 0, 2, 0, 0, 0, 1, 0, 0, 0]); | ||
| let expected: [u32; 16] = [ | ||
| 0xA87DBC7E, | ||
| 0x1787178C, |
There was a problem hiding this comment.
Instructions to reproduce would be nice. My hack is something like:
- Tweak the number of mixing steps to match (
sfc.cpp:92) - Compile:
for s in $(find src -name *.cpp); do g++ -I include -c $s; done g++ -I include -c tools/RNG_output.cppg++ *.o -o RNG_output./RNG_output sfc32 8 0x100000002 | hexdump
hexdumpis clearly not the right tool here; we want to read 32-bit LE values not 16-bit LE.
There was a problem hiding this comment.
I'll put together a procedure, in all honesty I just spit the values into a file that I then opened in a hex editor and copied in...
There was a problem hiding this comment.
Okay so the procedure I came up with is basically:
- Compile PractRand as specified in https://pracrand.sourceforge.net/installation.txt.
- For Sfc32 run
./RNG_output sfc32 76 0000000100000002 | od -j 12 -t x4 -v(skipping 12 bytes to account for the extra 3 mixing steps). For Sfc64 it's./RNG_output sfc64 176 0000000000000001 | od -j 48 -t x8 -v.
There was a problem hiding this comment.
Your link is wrong (it points at github).
Ah, I didn't know od.
* add PractRand-style seed_from_u64 implementations * Cargo.toml changes * TODO: seed_from_u64 tests, readme?
The readme links are partially dead for now, as the repo isn't part of upstream
rand_sfc/src/common.rs
Outdated
| // A decent-quality 64 bit linear congruential generator used to extend seeds. | ||
| // Parameters from M. E. O'Neill. | ||
| pub fn seed_extender_lcg (state: u64) -> u64 { | ||
| let multiplier: u64 = 9199940308585234877; | ||
|
|
||
| state.wrapping_mul(multiplier).wrapping_add(multiplier) | ||
| } No newline at end of file |
There was a problem hiding this comment.
Your code here is essentially a 64-bit LCG with no output permutation or truncation; it appears to have been copied from https://www.pcg-random.org/posts/does-it-beat-the-minimal-standard.html but misses the output function (right shift).
If I recall correctly, the output size being smaller than the LCG state is a key part of ensuring good statistical performance of LCGs — though this may be less important here where only a few output values are used; I don't know enough to say for sure.
However, I still don't understand why you don't want to use the default seed_from_u64 implementation which uses PCG32 — essentially a 64-bit LCG with 32-bit permuted output?
There was a problem hiding this comment.
I think I probably took the "we are committing to stability of its output" comment a little more seriously than I should have. If you think that using the default implementation is OK, then we can do that, and I agree that would be better.
As for the LCG:
Yes, that's where I got that LCG from, I would have put the link but I figured that mentioning the author's name was enough (as it's a bog-standard LCG, just with uncommon parameters), but I can add the link if you need. My motivation for just using an LCG was principally it needing fewer lines of code than a PCG.
The reason I skipped the output function (and only for Sfc64, Sfc32 effectively does use it) was that based on the testing that I did the quality of the generator was basically irrelevant, it just had to be something. So I figured using all the bits from an LCG would be good enough, as it would get run through the Sfc64 mixer anyway.
There was a problem hiding this comment.
I think I probably took the "we are committing to stability of its output" comment a little more seriously than I should have.
Sorry for not being clearer. No, I just wanted to make a note of this.
I would have put the link but ...
I just searched for the constant. Easy.
|
Just a little reminder: this PR is mostly ready, but needs a few clean-ups (see my last comment). |
|
Hi, apologies. I'll look at this tomorrow. |
|
Replaced by #72. |
Replaces rust-random#65 (only two small commits on top of @tertu-m's work). --------- Co-authored-by: tertu marybig <flameshadowxeroshin@gmail.com>
This adds an implementation of the SFC32 and SFC64 chaotic RNGs.
There's some work left to be done regarding docs and formatting but they should work; I would like feedback on that.
EDIT: I'm also not even sure if this would be useful. One advantage it does have, perhaps, is that it does not require multiplication at all.