Skip to content

Handle p=1 in NegativeBinomial::ln_pmf like Binomial - #470

Open
teddytennant wants to merge 2 commits into
statrs-dev:mainfrom
teddytennant:wrangle-nb-e
Open

teddytennant wants to merge 2 commits into
statrs-dev:mainfrom
teddytennant:wrangle-nb-e

Conversation

@teddytennant

@teddytennant teddytennant commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

NegativeBinomial::ln_pmf multiplies k * ln(1-p). At p = 1 and k = 0 that is 0 * (-inf), so pmf(0) is NaN instead of 1.

That is the empty product (1-p)^0. Binomial already special-cases p == 1 as a point mass at n rather than evaluating the same 0/0. Do the same here: at p == 1, ln_pmf(0) = 0 and every other k is -inf.

test_pmf_p_one / test_ln_pmf_p_one cover integer and non-integer r. The old test_is_nan expectations for those points are gone.

CARGO_TARGET_DIR=... cargo test --lib distribution::negative_binomial : 19 passed.

Fixes #463

Summary by CodeRabbit

  • Bug Fixes
    • Corrected negative binomial probability calculations when the probability parameter equals 1.
    • Zero-count results now return the correct finite value, while nonzero counts correctly indicate zero probability.

@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: adbbe123-4304-4c2e-a54a-f62516bedc4f

📥 Commits

Reviewing files that changed from the base of the PR and between 52248ee and 4b20665.

📒 Files selected for processing (1)
  • src/distribution/negative_binomial.rs

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

The change handles p == 1 directly in ln_pmf. It returns point-mass values for zero and positive inputs. PMF and log-PMF tests cover multiple r values.

Changes

Negative binomial boundary handling

Layer / File(s) Summary
Handle p = 1 in PMF calculations
src/distribution/negative_binomial.rs
ln_pmf returns 0 for x == 0 and negative infinity for positive x when p == 1. PMF and log-PMF tests validate these values across additional r values.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 4b206

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: day01

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: handling p = 1 in NegativeBinomial::ln_pmf, consistent with Binomial behavior.
Linked Issues check ✅ Passed The changes address issue #463. NegativeBinomial::ln_pmf now returns 0 for k = 0 and negative infinity for other k when p = 1, which makes pmf(0) equal 1. Tests cover integer and non-integer r values.
Out of Scope Changes check ✅ Passed The changes are limited to the p = 1 boundary case in NegativeBinomial::ln_pmf and related tests. No unrelated changes are present.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 1 files.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NegativeBinomial::pmf(0) is NaN when p is 1

1 participant