search

rust-random / rand

A Rust library for random number generation.
https://crates.io/crates/rand
Other
1.68k stars 431 forks source link

Fix PERT distribution for when `mode` is very close to `(max - min) / 2` #1311

Closed LukasKalbertodt closed 6 months ago

LukasKalbertodt commented 1 year ago

There is already a special condition for this case, as the general formula breaks for floats. However, the check uses == which is fishy for floats. This commit instead checks if the difference is smaller than the machine epsilon.

Without this commit, this returns an error (despite being totally valid parameters for PERT):

Pert::new(0.0, 0.48258883, 0.24129441)
LukasKalbertodt commented 1 year ago

Your previous suggestion makes the most sense to me:

let thresh = 2 * F::EPSILON * max(mu.abs(), mode.abs());

I think the problem with this is that mu can have an error that is not proportional to its magnitude. If the inputs of that calculation (e.g. min and max) are considerably larger than what mu ends up being, mu has an error that's proportional to those large inputs.

But... I've looked at this quite a while now and I'm not 100% sure about anything. It doesn't help that I don't know the semantic meaning of mu, shape or many other parts. So, I'm fine with merging the threshold calculation you mentioned. I pushed that now.

dhardy commented 6 months ago

Closing in favour of #1452.