Deprecate rand::rngs::mock module and StepRng

[dhardy]

• Added a CHANGELOG.md entry

Summary

Deprecate rand::rngs::mock module and StepRng

Motivation

We have had many reports of unexpected outputs when using StepRng with other random algorithms (especially Uniform / random_range): #628, #1020, #1303, #1578, #1633. These reports are due to people assuming that StepRng::new(0, 1) will immediately cycle over all possible outputs of a small range; it won't because our uniform range algorithms use variations on widening multiply, not division-with-remainder.

Moreover, our algorithms should only be expected to yield outputs of the expected distribution when fed random inputs. As such, the only expectation which may be met when using a StepRng as input is that all outputs will be valid outputs. (Unless someone were to test all possible outputs until StepRng loops — get back to me when your test suite has finished counting to u64::MAX! But even then, assumptions wouldn't hold to algorithms potentially consuming multiple random values in sequence.)

Further motivation is that we can't think of a good reason to use StepRng over something like SmallRng Pcg* or Xoshiro*, which are easy to implement and yield pseudo-random output. The rand tests show some good reasons to want a StepRng: in order to test how a randomised algorithm behaves with fixed input. This of course only has value where the exact behaviour of the algorithm is known, which is usually limited to the test code in the module where it is defined since this behaviour is usually not specified.

In other words, StepRng should not be used except when testing a randomised algorithm implementation (thus all the issues linked above are mis-uses).

Details

StepRng and the mock module are deprecated, awaiting removal in the next breaking release.

[newpavlov]

I think we can simply remove benchmarks for StepRng.

[dhardy]

I think we can simply remove benchmarks for StepRng.

Yes...

The more questionable case are the tests in src/rng.rs and src/rngs/reseeding.rs. Some of those appear to want a ConstRng.

Then there is a usage in uniform_float.rs which actually uses stepped output (lowering_max_rng).

Maybe we should keep StepRng (but make it private to the crate)?

Note: I updated the motivation above (last two paragraphs). I also note one usage in rand_distr (testing the Triangular distribution), but we don't need to care much about that (we could copy the StepRng code to that crate).

[newpavlov]

The easiest solution would be to just use #[allow(deprecated)] on the affected test modules. In future it may be worth to add const initialization methods to some of small PRNGs and rewrite tests accordingly

[dhardy]

add const initialization methods to some of small PRNGs

This is not why those tests use StepRng. They use it in order to feed specific inputs to the random algorithms being tested. This use-case is a valid use of StepRng, but one that's only applicable when writing random algorithms (as in rand and rand_distr, though the latter only has one use-case).

[newpavlov]

They use it in order to feed specific inputs to the random algorithms being tested.

For this we can use an internal mock RNG which just reads values from a static array. Something like this:

pub MockRng32 {
    values: &'static [u32],
    pos: usize,
}

Or indeed keep StepRng and make it private (which is equivalent to removal for external users).

[dhardy]

Tests still generate warnings like this:

warning: use of deprecated constant `rngs::mock::tests::test_bool`
   --> src/rngs/mock.rs:99:5
    |
99  | /     fn test_bool() {
100 | |         use crate::{distr::StandardUniform, Rng};
...   |
105 | |         assert_eq!(&result, &[false, true, false, true, false, true]);
106 | |     }
    | |_____^
    |
    = note: `#[warn(deprecated)]` on by default

There's already #![allow(deprecated)] at the top of the file; additional annotations don't help. I'm guessing this may be a side effect of the #[test] annotation.

Shall I just delete the tests?

[newpavlov]

I think, yes. It does not look like such tests add anything useful (I assume stability of distributions using a proper PRNG is tested in relevant modules).