Uniform<f32/f64> should honor high/low limits

[sicking]

We should require that all Uniform distributions stick to the high/low limits, even in the face of rounding issues. Users are likely to see unexpected and hard-to-reproduce bugs if the returned value on rare occasions will be outside of the specified limits.

One example that I ran into was while implementing choose_weighted for dhardy#82. If the generated number can be slightly higher than high, that means that we risk iterating through the whole item-set without selecting any item to return.

The simplest approach is to simply check the generated value against high/low and loop if it doesn't fit withing the desired boundaries. However that runs into problem when scale rounds to infinity, since then we'll just loop forever.

Looking into some alternative approaches instead.

[dhardy]

Probably either rejection sampling or clamping will be needed...

I'd rather see a complete solution for one particular problem (e.g. choose_weighted), and then discuss whether to adjust Uniform sampling. Use guard functions for the sampling if necessary.

[pitdicker]

I'd rather see a complete solution for one particular problem (e.g. choose_weighted), and then discuss whether to adjust Uniform sampling.

Just wanted to write the same. Good that you thought about this corner case! But I can see other users complaining about the performance drop in gen_range, for a reason they don't cure much about. Maybe it is better to do the rejection sampling outside of gen_range?

@dhardy It can be a simple function, but what do you think of some method to make rejection sampling extra simple (i.e. that allows users to not write the loop)? Not thought it through though...

[dhardy]

So something like Uniform::new(low, high).sample_strict(&mut rng)? I suppose that could work.

[pitdicker]

I was thinking more in the direction of:

pub trait Distribution<T> {
    /* ... */

    fn rejection_sample<R: Rng + ?Sized>(&self, rng: &mut R, min: T, max: T)
        -> T where T: PartialOrd<T>;
}

let val = Uniform::new(low, high).rejection_sample(&mut rng, low, high);

Hmmm, doesn't look great. Maybe some iterator adapter:

let val = Uniform::new(low, high).sample_iter(&mut rng).accept_if(|x| x >= low && x < high);

Also ugly. And does the same as:

let val = Uniform::new(low, high).sample_iter(&mut rng).filter(|x| x >= low && x < high).next();

[dhardy]

Yes, a new method on Uniform was the only way I could think of not having to specify the limits twice.

Not sure if it's worth it, but it could be an unobtrusive layer on top of the current sample at least.