Rand crate revision (pre-stabilisation)

[dhardy]

Summary

Evaluate options for the future of rand regarding both cryptographic and
non-cryptographic uses.

There is a strawman revision which implements or demonstrates many of the changes
suggested in this document, but is not entirely in-sync with the suggestions
here.

Most current work is going on in the issue tracker and pull requests on this fork.

• rand-core RFC (subset; intended to be merged first)
• RFC text (not fully up to date)
• Crate evaluation thread (original topic)
• Strawman design PR
• Strawman revision code
• Strawman design API doc

[Lokathor]

SeedableRng only really needs one of those two options, not both. Probably best to keep the constructor version. Even when you consider the Ownership/Ref/MutRef stuff that is special to Rust, there's no situation where reseed would do something that you can't also do with from_seed.

There's no reason for RNGs to not also have an .into_iterator() method that gives a GeneratorIter or somesuch like that. There's no lifetime issue if you consume the generator.

If we're going to have weighted_bool we might as well have something like float_chance as well (needs a better name maybe). Rolls f32 and f64 values and gives a bool of if they land in the percentage specified, eg: 0.32 is 32% chance of being true. It's easy to do, but also the sort of thing you'll be implementing over and over, so we should just do it properly once in the library itself.

Obviously I support the simpler version of a Rand trait. You can build all the more complex stuff over top of it, but you can't get back down to that level if you start with the complex version. We should provide our tools in a way that lets people use only as much as they need, if possible.

As to using a Range struct for a speed boost by pre-computing a range, that's cool and people should do that. You can do that even with the simplified Rand version. The struct wouldn't implement RangedRand, you'd just use a method on the struct that's not part of any trait at all. Some sort of RangeFoo type. It has to be hand-specialized to each type, probably. The thing with ranges is that for all the primitives T you have to do math relating to T::MAX to compute the range info properly, which I don't think you can do generically at the moment. There's also max_value(), but that's the same problem. If we want to go in on a group of range structs we can drop the RangedRand trait entirely and still keep the simple Rand trait at least (the range types could then be built for any type that implements Rand).

If we're having userspace PRNGs thare are not CSPRNGs, then we should absolutely add the PCGen and Xoroshiro PRNGs. Xoroshiro is even faster than pcg, but it's got

Edit: it's got some unfortunate trouble with the lowest bit, so we'd have to be sure to override next_bool or equivalent with that generator.

[dhardy]

SeedableRng only really needs one of those two options, not both. Probably best to keep the constructor version.

Agreed

There's no reason for RNGs to not also have an .into_iterator() method that gives a GeneratorIter or somesuch like that. There's no lifetime issue if you consume the generator.

I think there still are lifetime problems actually, if you want std::iter::Iterator. Otherwise you need a streaming iterator.

If we're going to have weighted_bool we might as well have something like float_chance as well (needs a better name maybe).

I think you mean the Bernoulli distribution.

Obviously I support the simpler version of a Rand trait.

Yeah, I do too now; it's simpler and all the distribution stuff is available anyway. Even the simpler version is redundant against Default, but probably best to keep both. @aturon suggested this version so I'll wait for some feedback from them now.

As to using a Range struct ...

Sounds like you're not too familiar with the two range modules ... I also thought about only having specialised versions (RangeInt, RangeFloat, etc.), but probably better to hide that as implementation details. Which is what range2 tries (awkwardly) to do.

If we're having userspace PRNGs thare are not CSPRNGs, then we should absolutely add the PCGen and Xoroshiro PRNGs.

Sounds like a good plan to me. But I'm not sure everyone agrees, hence the question over splitting the crate into a minimal core and extras or not. Did you finish your last sentence BTW?

[Lokathor]

Whoops. Fixed. I will reply a little more when i have more time later.

[Lokathor]

I think there still are lifetime problems actually, if you want std::iter::Iterator. Otherwise you need a streaming iterator.

No I'm talking about IntoIterator, the version that consumes an owned value. No lifetime issue because there's no lifetime because there's no reference. It's not something that will be used a whole lot, but it should be available if you have ownership and Rust doesn't allow orphan instances, so we'll have to include it in the crate.

I think you mean the Bernoulli distribution.

Yeah, neat. This is good. Might want a name that people not familiar with math would recognize a little more, but that's fine. I guess you can just say in the rustdoc what it does.

Sounds like you're not too familiar with the two range modules .

I was thinking mostly about the existing rand::distributions::range::Range. Hadn't seen the latest range update in the strawman proposal, if they're different at this point. As much as I hate trait soup, looking at range2 just now it feels like a good justification for a mild amount of it. It's very similar to how I did ranges for a game project over the weekend. Though, in my case the ranges produce values across [low,high] instead of [low,high), so that a 6-sided die is written ::new(1,6) instead of ::new(1,7). Either way, you use them about the same, range.withGenerator(generatorMutRef). I think that From<std::ops::Range> might be appropriate, or at least alternate constructors that let you pass in a std::ops::Range if you want.

The important part that we should keep in mind is in how easy it is to make new range implementations if you don't have a macro doing it for you. The macro can simplify the eight integral types and two float types, but it clearly wouldn't know how to handle a brand new GLRGBColor type (which would be three f32 values, each somewhere in the [0.0,1.0] range), so someone is going to have to write some instances by hand. How easy of a time will they have? Probably not too hard if we write clear docs on how to do it. It looks like, with the range2 version, you make a SampleRange and then give that a RangeImpl and then that lets you do stuff, but I'm not totally clear.

[newpavlov]

Regarding relation between CryptoRng and Rng. How about we define CryptoRng in a separate crate (crypto-rand?) with the single method fn fill_bytes(&mut self, dest: &mut [u8]) -> Result<()>. Next we import this trait into rand crate and do impl CryptoRng for Rng { .. } which will panic in case of error returned by CryptoRng. IIRC there is no obstacles to implementing that.

This will allow us to keep crypto-rand crate very small (big plus for security reviews) and to keep all convenience methods associated with Rng trait.

As for splitting between crates, I personally think we should not keep algorithms in rand and instead publish them as separate crates with links to them in the rand documentation.

Small suggestion regarding SeedableRng, I think it will be more convenient to use associated type for Seed:

pub trait SeedableRng: Rng {
   type Seed;

    fn reseed(&mut self, Seed);
    fn from_seed(seed: Seed) -> Self;
}

[dhardy]

No I'm talking about IntoIterator ...

You still need to implement Iterator for a Rng somehow. Go ahead, and try for yourself if you don't believe me.

Regarding Bernoulli and other small distributions, this is how I've done it before. Here we could make use of the Distribution trait and do: let sample: bool = Bernoulli(0.5).sample(rng); or Bernonulli::new(0.5).sample(rng) (the latter allows bounds-checking on construction). I'm not sure of another good name besides P.

I don't know if you want to use Range to generate a random RGB value anyway, because you probably want more than just "low" and "high" parameters. So implement a new RGBRange or HSVDistribution or something? I'll leave that to you.

@newpavlov the crypto-rand crate seems like a good idea to me. I think we should also put OsRng in that crate. @briansmith what do you think of this? You could probably just publish your rand module as crypto-rand; SystemRand and OsRng are basically the same thing?

What was your other point @newpavlov, put Rng and all distribution code in rand but no PRNGs at all? I guess that does work, and saves questions of which PRNGs to include. I'm not 100% sure about this, but it could work (like log needs another crate to generate output).

@newpavlov using an associated type means each PRNG can only be seeded from one type... probably a good idea actually. Also see @Lokathor's point about removing fn reseed(&mut self, seed: Seed).

[Lokathor]

You still need to implement Iterator for a Rng somehow. Go ahead, and try for yourself if you don't believe me.

With my limited trait-fu I only know how to implement Iterator for Rng:

impl Iterator for Rng {
    type Item = u32;
    fn next(&mut self) -> Option<Self::Item>{
        Some(self.next_u32())
    }
}

but I do not know how to implement IntoIterator for an arbitrary Rng value. I don't even know how to specify the type bounds for that one properly. Associated types are unfortunately new to me compared to the rest of the traits system, my Haskell is showing.

I at least know how to do it for specific structs that implement Rng:

pub struct RngIter<T:Rng>(T);

impl <T:Rng> Iterator for RngIter<T>{
    type Item = u32;
    fn next(&mut self) -> Option<Self::Item>{
        Some(self.0.next_u32())
    }
}

// here, PCGen32 is a type that implements Rng
impl IntoIterator for PCGen32 {
    type Item = u32;
    type IntoIter = RngIter<PCGen32>;
    fn into_iter(self) -> Self::IntoIter{
        RngIter(self)
    }
}

And then I got stuck.

Regarding Bernoulli and other small distributions

That sounds... overkill? In this specific case, you're always using the HalfOpen range deal, [0,1), so I think there's no construction needed.

fn chance<R:Rng>(rng: &mut R, chance: f32) -> bool {
    let roll: f32 = rng.gen();
    roll < chance
}

This is valid with the current rand crate.

I don't know if you want to use Range to generate a random RGB value anyway

Merely an example to illustrate the fact that we have to have it be clear how you'd make new instances for new types. Saying "if you have a weird type that isn't really like a number you'll maybe have to make up stuff yourself" is totally a valid decision, as long as we write that down in the docs.

[dhardy]

Oh, you want an iterator which yields a stream of u32 values? In that case you can do iter(rng).map(|rng| rng.next_u32()) with my rand revision. Sorry, I thought you wanted an iterator which returned Some(&mut Rng) on each next() call.

Using a trait is overkill when a simple function suffices? I guess. Does it matter? One of Rust's great strengths is that you don't pay for this kind of stuff, except via a tiny bit of compiler time. (In C++ you paid with your sanity when trying to understand compiler errors — Rust at least mostly fixed that. I don't know Haskell.)

[KodrAus]

Requiring Debug definitely seems odd to me.

[KodrAus]

Without understanding too much more, it feels out-of-scope for Rng to be dictating the way implementations are constructed. What's the motivation around new_from_rng?

[dhardy]

new_from_rng replaces impl Rand for XXX from the current crate; e.g. see doc for ChaChaRng.

This is currently used to construct RNGs from OsRng. Potentially it could also be used to construct RNGs from other PRNGs, but that's not always a good idea, and isn't used within the rand crate anywhere, so new_from_rng could be removed now that RNGs have new seeding from OsRng. This would remove a footgun but also remove functionality which might sometimes have a legitimate use (I've wanted to try making a deterministic parallel simulation a couple of times).

[Lokathor]

impl Rand for PRNG is pretty easy to understand, you use let mut newGen: Foo = rng.gen() and the generics take care of it. I'm not sure why you'd want to remove it exactly.

On the other hand, most PRNGs will also be SeedableRng, and any SeedableRng is likely to be seedable from some type that has a Rand instance. In that case you'd just write Foo::from_seed(rng.gen()) to get the same effect.

[KodrAus]

Ah gotcha. Hmm, I'm in two minds about using a single trait for generating random values vs bootstrapping (is there a better term for that?) other generators.

At least it's a bit surprising to me on my first pass through the API. I like the idea of keeping those concepts separate by default, as the RFC suggests.

If we were going to stick with Rand for constructing these generators from another one then I think the docs should call out that pattern explicitly.

[KodrAus]

So to clarify, I like removal of Rand, and by extension using it to build rngs.

I don't think we need to explicitly recommend generators follow a particular pattern and leave it up to the specific implementation to know what makes sense and what will be consistent with the generators provided by rand.

[Lokathor]

One thing that @dhardy said: you can put rustdoc on the methods of a trait implementation for a type, and when you look at that type's implementation list as you scroll down the page that rustdoc will appear on the correct methods. Im not sure if they meant something else by "Rust not allowing documentation of impls" or not.

[dhardy]

let mut newGen = NewGenType::from_seed(oldGen.gen());

I suspect this won't work for many RNGs: the implementations of SeedableRng for ISAAC and ChaCha take a slice to seed from. Slices don't have a specified length in the type (like for example [u8; 4] does) and are not implemented directly by Rand or equivalent.

[Lokathor]

I suspect this is how the coding session would go if that error came up:

let mut newGen = NewGenType::from_seed(oldGen.gen());

Compilation error? What the crap? Fine, Rust, if you want to be difficult I'll spell it out for you.

let seed: [u8;10] = oldGen.gen();
let mut newGen = NewGenType::from_seed(seed);

Ha, look, I'm smarter than a compiler.

The point is that if a user gets it into their head to make a generator from a generator, they're going to fight to do that more than they're going to look up in some docs somewhere why that's not a good idea. And if array10 (or whatever size) doesn't have a random instance then they'll make an array with zeroed and fill that up using a loop. They'll do whatever it takes to get the job done more than they'll stop and reconsider. This happens all the time with programmers, and this happens more often with people that are new to a field/library/problem domain/etc. because the person doesn't understand they they have an XY Problem on their hands.

It's just human nature, and you can't really fix it, but you can anticipate it and plan for it by making sure that the path of least resistance for this sort of obstinate scenario is one that's actually a safe solution.

[dhardy]

Why would someone find SeedableRng::from_seed but not NewGenType::from_rng, when they are trying to create a NewGenType?

[Lokathor]

I agree with you that we should suggest from_rng be made when possible. KodrAus was the one against the idea of even suggesting that. Sorry for the confusion there.

[Lokathor]

Oh! Ha, an Iterator over potential RNG states or something like that would be very different indeed.

As to traits compared to simple functions: I don't mean that there is a runtime cost on the program's execution speed, I mean that there is a cognitive cost to the programmer. Every element of a program should be exactly as simple as possible for it to accomplish its task. "A designer knows he has achieved perfection not when there is nothing left to add, but when there is nothing left to take away.", and all that sort of stuff.

Last week we had this blog post in the news, and it's pretty much all true. A library can be as clever and smart as it wants to be but it's useless when people can't actually understand it and use it. Libraries like rand, that are used by so many people, should strive to be as exceedingly simple as possible. I'd seriously argue that the best crate in the entire Rust ecosystem is lazy_static. It solves a common problem in the easiest to understand way possible.

[dhardy]

Small suggestion regarding SeedableRng, I think it will be more convenient to use associated type for Seed

It turns out that both ISAAC and ChaCha RNGs accept a slice for initialisation. A slice (&'a [u8]) must have a lifetime, so e.g. to impl SeedableRng for Isaac64Rng with an associated type, the lifetime must either be part of the SeedableRng trait (template parameter) or part of the Isaac64Rng struct. Since the data passed should definitely not be bound to the lifetime of the RNG, SeedableRng needs a template parameter, hence there's no point in using an associated type (unless one specifically wants to prevent multiple implementations, which doesn't make much sense).

Regarding removing fn reseed(&mut self), this would be easy enough but the reduction in complexity doesn't seem significant.

[newpavlov]

@dhardy

What was your other point @newpavlov, put Rng and all distribution code in rand but no PRNGs at all?

Yes this one, I think that such approach is quite common, with log and http examples come first to mind.

Also see @Lokathor's point about removing fn reseed(&mut self, seed: Seed).

If reseed is equivalent to from_seed which works in-place, then I agree with @Lokathor and think it's a redundant method. Although maybe for some PRNGs it could be useful if state is quite large and allocated on the heap. Maybe make this method with default implementation defined over from_seed method? So if needed PRNG implementation could overwrite it with a more efficient implementation.

I had in mind something like FeedableRng trait, which I've mentioned in the internals thread:

trait FeedableRng {
   type Entropy;

   fn feed_entropy(&mut self, entropy: Entropy)
}

Not sure if SeedableRng bound is needed here, as this trait can be defined for user space CryptoRng and IIRC even for OsRng on Linux.

[dhardy]

@newpavlov FeedableRng seems good, but I'd probably make the Entropy type a type parameter for the same reasons as with SeedableRng above. @briansmith what do you think?

---

@Lokathor what do you think of the following? Does it reduce cognitive load enough that using distributions is now okay?

In the lib (feel free to suggest a new name):

pub trait SampleFromRng {
    fn sample<T, D: Distribution<T>>(&mut self, distr: D) -> T;
}

impl<R: Rng+?Sized> SampleFromRng for R {
    fn sample<T, D: Distribution<T>>(&mut self, distr: D) -> T {
        distr.sample(self)
    }
}

In user code:

use rand::{thread_rng, SampleFromRng};
use rand::distributions::{Uniform, Range, Exp};

fn main() {
    // use with a static Rng type:
    let mut rng = thread_rng();
    
    let _a: u32 = rng.sample(Uniform);
    let _b = rng.sample(Range::new(-2, 15));
    
    // or use with a dynamic Rng type:
    let mut rng: &mut Rng = &mut thread_rng();
    
    let _c = rng.sample(Exp::new(2.0));
}

I guess we could add methods like fn gen<T>(&mut self) -> T and fn sample_range<T>(&mut self, low: T, high: T) -> T to SampleFromRng, but I'm not sure that adding extra wrappers like this is really a good idea.

[newpavlov]

@dhardy
You are right, use of type parameters instead of associated types is good with me.

[burdges]

Just another thought on error handling here: #2191

[vitiral]

I'm requesting that rand::sample() be fallible in this issue: rust-random/rand#194

[arthurprs]

I did miss a lot of comments but I couldn't parse a conclusion from the rfc text.

Did we reach a consensus regarding providing only opaque RNGs (std_rng, weak_rng, thread_rng, osrng,....) (stable across major versions)?

[dhardy]

Not really. There's some discussion here. There has also been quite a bit of talk about reproducible generators; these would obviously require names (e.g. ChaChaRng) but there's no hard requirement these are kept in the rand crate.

[arthurprs]

Thanks. IMHO we shouldn't expose anything named. StdRng and WeakRng can be seedable/reproducible anyway.

[dhardy]

They can't be reproducible without adding a major restriction: not being able to replace the implementations with better PRNGs in the future.

[arthurprs]

You can always guarantee they're stable across major versions.

[dhardy]

Isn't that what I just said above? But why? We've already got a proposal to replace StdRng with a better generator.

Unless you mean allow breakage. But again why? Someone writes a scientific simulator, does an experiment, publishes a result, along with a link to let other people reproduce output. Someone else writes a game with procedurally generated maps. Symmetric encryption works the same way: permute using a fixed pseudo-random sequence (although obviously people select named CSPRNGs for that, like ChaCha20). Queue a major version update, and suddenly people have trouble reproducing the simulation result and players complain that their maps changed, and for no good reason. Reproducibility means guaranteeing to produce the same output.

[arthurprs]

I'm not sure were we are misunderstanding each other but let's try again. I wholeheartedly agree that reproducibility is important, so let's get that out of the way.

I'm arguing that rand shouldn't provide specific implementations like xoroshiro or chacha, specially because the optimal choice for StdRng, WeakRng, .... tend to change over time for various reasons. Users will be able to achieve reproducibility using the provided opaque generators as long as they're continue using the same major version 1.X, 2.X, ...

[dhardy]

So you're suggesting a half-way house? Well I have argued elsewhere that StdRng should not be seedable or serialisable, because it mis-leads users into assuming those generators are reproducible. (That is, StdRng can be automatically seeded with fresh entropy but we do not provide any way of constructing it with reproducible output.) Same for WeakRng if we even provide that.

That still leaves open what you suggest, not having named generators like ChaChaRng in rand, which may be a good idea.

Sorry, been through this several times but too lazy to find the links because most of it is buried in the middle of a long comment thread like this one. Much better if you could open an issue to discuss stuff like this.

[Lokathor]

@arthurprs In the world of Rust, going from 0.x to 1.x is usually seen as the last breaking change you should ever make. The mentality is that you should never need to go to 2.x, and certainly not for at least a few years after 1.x came out.

[dhardy]

@Lokathor there are significant disagreements whether Rust 2.x should ever exist. And while it would be feasible to release rand 1.0 not expecting anything to ever be broken, I can't see a good reason we shouldn't plan to allow significant low-friction upgrades in the future. Oh, and a lot of people consider "a few years" a short time-frame, outside (and inside) the tech world.

[arthurprs]

@dhardy Yes. If the user have a stronger reason to get algorithm X it can just use its crate.

Consider WeakRng for example, there were almost half a dozen proposals to improve it. One can argue that the initial choice wasn't the best but its bound to happen again.

If that kind of thing happen again the maintainers will have 3 choices (assume the PR has merits):

1. Reject the PR (for reason X) and politely ask the author to push it to crates.io
2. Accept the PR adding generator G to rand
3. Accept the PR adding generator G to rand and mark the other(s) deprecated

Note that 2 and 3 are potentially duplicating something from crates.io anyway, but with a blessing.

[dhardy]

@arthurprs what you say makes sense, so long as (a) the external crates aren't huge and (b) the external crates are sufficiently trustworthy (e.g. it may not be desirable to include a crate from an unknown author).

But this is an important issue, and one I've already stepped around. Can you make a new issue to summarise please? (Easier for others to read.)

[Lokathor]

dhardy I was trying to agree with you and somehow you still ended up disagreeing with me ?_?

[dhardy]

@Lokathor sorry; I frequently disagree with everyone ;-) No offence intended there; I like to break arguments down into small enough pieces that we can work out what we do/don't agree on :-)

[Lokathor]

Since rand_core seems to have a crate now, I'll mention a few things about that:

• SeedableRng does not need a reseed method as far as I can tell. There's not much justification I can see for it. If you have a &mut to some RNG, you can just use from_seed and assign that into your &mut. Making two methods seems like it just leads to a lot of weird duplication. This isn't really like Clone, where some values are going to be very big and use a lot of heap and maybe you use clone_from to avoid an allocation. An RNG will generally be quite small (16 bytes or less) and live entirely on the stack.
• I'm not clear on the value of SeedFromRng. The note says that it's distinct because of some sort of dynamic dispatch issue, but it seems like (once we have distributions sorted out) we can use a generator to produce any arbitrary seed value (eg: rng.gen() as the current version of rand calls it) and then feed that into from_seed. Can you talk about why that wouldn't be fine?
• When might we see the 0.0.1 version cleaned up a bit and released as at least a 0.1.0 version? It's getting hard to keep track of all the rand update things at this point, there's so many of them. I'd like to push my own randomization stuff to crates.io but with rand_core only kinda half-done it's not as appealing to design against.

[nagisa]

There's not much justification I can see for it.

It could be implemented as a method that adds entropy, rather than replacing it.

[Lokathor]

That would be some entirely other trait. SeedableRng is about being able to track, at the type level, what the seed needed for a generator is. Being able to feed some additional amount of entropy to a generator would be another trait.

[dhardy]

We already have an issue on seeding RNGs: dhardy/rand#18

[aturon]

I'm closing out this RFC, as work is instead happening directly in the rand repo.