Handle p=1 in NegativeBinomial::ln_pmf like Binomial - #470
teddytennant wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe change handles ChangesNegative binomial boundary handling
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to This change makes the negative-binomial distribution return valid point-mass values at p=1 instead of NaN, with boundary behavior covered for PMF and log-PMF. It is ready to merge. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
NegativeBinomial::ln_pmfmultipliesk * ln(1-p). Atp = 1andk = 0that is0 * (-inf), sopmf(0)is NaN instead of 1.That is the empty product
(1-p)^0. Binomial already special-casesp == 1as a point mass at n rather than evaluating the same 0/0. Do the same here: atp == 1,ln_pmf(0) = 0and every other k is-inf.test_pmf_p_one/test_ln_pmf_p_onecover integer and non-integer r. The oldtest_is_nanexpectations for those points are gone.CARGO_TARGET_DIR=... cargo test --lib distribution::negative_binomial: 19 passed.Fixes #463
Summary by CodeRabbit