SeedableRng cannot be implemented for large RNGs using the recommended seed type

[vks]

For example, xorshift1024* has a recommended [u8; 128] seed type, for which std::convert::AsMut<[u8]> is not implemented, making it impossible to implement SeedableRng. This also affects common RNGs like the Mersenne Twister.

[vks]

I think either the documentation should discuss how to implement SeedableRng for large RNGs or the trait should be improved to support [u8; N] for N larger than 32.

[dhardy]

8 × 32 = 256 bits which is more than enough for most purposes. Some key expansion algorithm should be used internally for larger state sizes I think. If you want compatibility with initialisation as used in another implementation, then I think a custom constructor is the best option.

[pitdicker]

I really don't like that AsRef and AsMutare not available for common powers of 2. I am almost thinking if we should provide some array wrapper, like I added for ISAAC's result type in #325.

[pitdicker]

But as you say, @dhardy, 256 bit's should just be enough.

[vks]

If we decide to do drop support for large RNGs, I think we should document the limitations of SeedableRng and suggest some workaround.

Using some key expansion is not really an option if reproducibility across implementations is needed. A custom constructor is not ideal, because it does not support from_rng.

[burdges]

We know const generics will solve this. If we need this sooner, then we should do a PR or RFC for the needed sizes as "preparation for const generics". It might encourage them to deliver const generics sooner. If it can wait till 2019 then just let it wait.

[vks]

Ok, I think the best workaround is to wrap the large seeds in a newtype and to implement then required traits by hand. No custom constructor required. It should be enough to update the documentation. I will submit a pull request.

[pitdicker]

It should be enough to update the documentation. I will submit a pull request.
👍

Does anyone feel adventurous to make a PR to the rust repo to support some more common powers of two for AsRef and AsMut?

[burdges]

Right, new types are beneficial anyways, so maybe something like:

"We currently recommend that SeedableRng::Seed types be special purpose "newtypes" wrapped in a tuple struct, like pub struct XorShift1024Seed(pub [u8; 128]);. This practice encourages PRNG users to read any documentation relevant to seeding, leaves rand more flexibility to possibly support multiple seed types in the future, and supports PRNGs with large seed types."

[vks]

In our case we also need `Default`.

…

On Fri, Mar 30, 2018, 15:19 Paul Dicker ***@***.***> wrote: Does anyone feel adventurous to make a PR to the rust repo to support some more common powers of two for AsRef and AsMut?

[dhardy]

@vks can I close this? Or do you want to make a documentation PR?

This is only a temporary problem anyway until const generics support is ready.

[vks]

I will make a PR. This is a papercut until const generics are in, which will at least take a year. The newtype pattern is also an elegant solution to avoid panicking in `from_seed` by making invalid seeds unconstructible.

…

On Wed, Apr 4, 2018, 12:04 Diggory Hardy ***@***.***> wrote: @vks <https://github.com/vks> can I close this? Or do you want to make a documentation PR? This is only a temporary problem anyway until const generics support is ready. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#354 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACCtLApVjeVqXIWFlExk6cWkvzxzBJfks5tlJq8gaJpZM4TATQi> .

[dhardy]

Good.

The doc on from_seed already suggests using replacement seeds instead if the input is not valid.

[vks]

@burdges I did not suggest to always use newtypes, because we are not doing it in Rand. If we decide to suggest this, I think we should also do it ourselves.