Fix broken negative binomial sampling #328
Conversation
Co-authored-by: bbbruce <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #328 +/- ##
==========================================
+ Coverage 94.40% 94.93% +0.52%
==========================================
Files 59 59
Lines 13432 13448 +16
==========================================
+ Hits 12681 12767 +86
+ Misses 751 681 -70 ☔ View full report in Codecov by Sentry. |
|
Thanks for catching this! |
| @@ -487,4 +487,25 @@ mod tests { | |||
| let sf = |arg: u64| move |x: NegativeBinomial| x.sf(arg); | |||
| test_absolute(3.0, 0.5, 5.282409836586059e-28, 1e-28, sf(100)); | |||
| } | |||
|
|
|||
| #[test] | |||
There was a problem hiding this comment.
Thanks for seeding so we don't have a flaky test! Would you be able to wrap this test with a cfg so that it only compiles with the rand feature?
| let theoretical_mean = dist.mean().unwrap(); | ||
| let theoretical_variance = dist.variance().unwrap(); | ||
|
|
||
| assert!(prec::almost_eq(sample_mean, theoretical_mean, tol)); |
There was a problem hiding this comment.
I have an optional suggestion here, would you use the hypothesis testing module to verify this sampler? I'd use chi square GoF. If not, I was thinking of adding these kinds of tests for sampling over time.
There was a problem hiding this comment.
This is a good idea - will keep in mind to add over time as possible
|
The only failing test is an unrelated one we're working on in #325, thanks for contributing a fix! |
Description
The negative binomial distribution sampler is not working because
gammais parameterized to sample usingraterather thanscale. The original input was the scale as a function ofself.pleading to incorrect samples for the specified distribution. Thesample_uncheckedis now supplied with the rate calculated fromself.pand a test is provided to ensure behavior is in agreement with the theoretical mean and variance of the specified distribution.