robojeb / pcg_rand

Apache License 2.0
13 stars 3 forks source link

First return from gen() on new_unseeded() is 0 #6

Closed BartMassey closed 6 years ago

BartMassey commented 7 years ago

Here's a program:

extern crate rand;
extern crate pcg_rand;

use rand::Rng;
use rand::XorShiftRng;
use pcg_rand::Pcg32;

pub fn main() {
    let mut xorshift = XorShiftRng::new_unseeded();
    let mut pcg = Pcg32::new_unseeded();
    let xv: u32 = xorshift.gen();
    let pv: u32 = pcg.gen();
    println!("{} {}", xv, pv);
}

And here's its output (at least on my box):

3690029583 0

While the pcg output is technically ok — the generator is unseeded, and so can start the sequence where it likes — I'd strongly prefer that the first output of an unseeded generator still "look random" as with xorshift. Fix should be as easy as a call to gen() inside new_unseeded().

robojeb commented 6 years ago

@BartMassey

Hi, sorry for being a bad maintainer, I am not quite used to being in charge of a project in the public eye.

I am not entirely sure how I feel about this, I think that it makes sense to have "bad" output from an unseeded RNG. Really none of these should be used from their un-seeded state, I have mostly provided it so that if for whatever reason someone needs to split allocation and seeding they can do that.

What use-case do you have for an unseeded generator?

BartMassey commented 6 years ago

No prob — glad to hear from you!

An unseeded generator is for cases like testing where one really doesn't care about predictability, but just wants an uncorrelated sequence. Most generators in most languages will give a perfectly reasonable sequence if used with the default seed (as xorshift does), so the behavior here is a bit surprising. If you think that your generators should not be used with a default seed, it would probably be best to both document this constraint and have the generator panic if used unseeded.

robojeb commented 6 years ago

That makes sense to me.

It looks like in xorshift they just picked some suitable random numbers (eg https://xkcd.com/221/) and return that. Though they have deprecated the function.

I am inclined to agree that providing a better initial seed is a good idea. Though I think it should be coupled with appropriate documentation pointing out how to properly seed the RNG.

BartMassey commented 6 years ago

Thanks much!

robojeb commented 6 years ago

Version 0.9.1 is now published with better defaults for new_unseeded.