rust-random / rand

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

Implement Distribution<u64> support for Zeta, Zipf #1516

Closed dhardy closed 4 weeks ago

dhardy commented 1 month ago

Closes #1323. I would have added usize and possibly u32 support, but @vks preferred not to, and this is reasonable.

These impls use @JamboChen's tests.

dhardy commented 1 month ago

It is unfortunate that there is no simple notation for type-ascribing in this case, aside from let x: f64 = distr.sample(rng);. (There were some unsuccessful RFCs.) Distribution::<f64>::sample(distr, rng) should be enough, but it's still unwieldy and not very intuitive.

To some extent this was baked into the original design: the result is a type parameter of the trait Distribution<T> not an associated type. Yet, other than Standard, this ability to support multiple output types hasn't been used much.

I feel that supporting only floats + u64 is the worst approach: it's enough target types to prevent inference in many cases, yet still doesn't support all potentially desired types (e.g. u32, usize).

benjamin-lieser commented 1 month ago

I feel that supporting only floats + u64 is the worst approach: it's enough target types to prevent inference in many cases, yet still doesn't support all potentially desired types (e.g. u32, usize).

I feel a silent saturation is often a serious bug and I don't want to make it too easy. For Zipf I would be ok, because the upper bound is part of the parameters, but for Zeta and Poisson I would not implement the smaller intergers. And I agree, the current approach seems to be the worst of both worlds, because getting the u64 impl does not do too much.

dhardy commented 4 weeks ago

Replaced by #1517