impl SeedableRng for StepRng #1496
Conversation
| type Seed = [u8; 16]; | ||
|
|
||
| fn from_seed(seed: Self::Seed) -> Self { | ||
| let seed_u64: [u64; 2] = transmute!(seed); |
There was a problem hiding this comment.
Concern: we should make results portable where (easily enough) possible. transmute! presumably does not alter the value, so we would need to use u64::from_le (or u64::from_le_bytes with as_chunks) here. Note that rand_core::impls::fill_bytes_via_next uses to_le_bytes so this would cancel out with a word-based-generator on a BE platform.
| let seed_u64: [u64; 2] = transmute!(seed); | ||
| StepRng { | ||
| v: seed_u64[0], | ||
| a: seed_u64[1], |
There was a problem hiding this comment.
Concern: if this were to randomly sample a 0, the output would be constant; if it randomly samples an even number, half the output values will be unreachable. I'm not entirely sure what behaviour people would expect here but suggest we do seed_u64[1] | 1.
There was a problem hiding this comment.
This makes me think this impl is a bad idea. The usecases for StepRng are probably the easy predictability and SeedableRng goes against this
There was a problem hiding this comment.
It may be that my concern is entirely invalid given that this is a mock RNG, but you may also be right.
|
Note: We should briefly document what this impl does, on the impl itself. |
|
Closing (along with motivating issue). |
CHANGELOG.mdentrySummary
implement SeedableRng for StepRng
Motivation
#1495
Details
This is not value stable across endianess, not sure if we need/want that here