rust-random / rand

A Rust library for random number generation.

https://crates.io/crates/rand

Other

1.67k stars 432 forks source link

Move ks #1525

Closed benjamin-lieser closed 4 days ago

benjamin-lieser commented 1 week ago

[ ] Added a CHANGELOG.md entry

Summary

Motivation

https://github.com/rust-random/rand/issues/1519

dhardy commented 1 week ago

Further note: could you split cdf.rs please? E.g.:

tests/ks/mod.rs provides the pub fns
tests/cdf.rs for most of the tests
tests/skew_normal.rs which is the only user of fn owen_t

(Optionally more segregation of tests.)

Part of the rationale is to make it easier to use this for additional tests — I'd like to add some for weighted sampling (for which we have several variants, so better to use a separate file).

benjamin-lieser commented 6 days ago

Thanks.

I'm not sure about depending on spfunc; that appears not to have been touched in three years and has one open issue noting an inaccuracy.

I see the point, but before we just copy pasted their zeta implementation

benjamin-lieser commented 4 days ago

It thought I put them with the other tests, so they get run on every supported platform. This would also run the MSRV tests and it seems simpler letting it pass these than try to avoid running them.

dhardy commented 4 days ago

Aha. I don't think that's necessary — the value-stability tests are there to ensure that we get the same results everywhere.

benjamin-lieser commented 4 days ago

Good point. I was also thinking if we should make the value stability tests more exhaustive by some automatic script to generate the tests (or reading values from file). Right now we only test a handful of values.

dhardy commented 4 days ago

More exhaustive value-stability tests would be good, yes. It wouldn't surprise me if there are some FP issues we've missed somewhere. In fact, we probably have given that tests pass both with and without std_math.