Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Characterization of negative binomial extended to the reals #115

Closed
boxtown opened this issue Jul 6, 2020 · 3 comments · Fixed by #164
Closed

Characterization of negative binomial extended to the reals #115

boxtown opened this issue Jul 6, 2020 · 3 comments · Fixed by #164

Comments

@boxtown
Copy link
Collaborator

boxtown commented Jul 6, 2020

See 6b2c726#diff-8f4094da35079696cbcdd4a269333ad0R101 for a little more context.

We're currently allowing the negative binomial distribution to accept positive real r values as it seems to be a common use case. However, this causes the CDF to behave continuously rather than like in other discrete distributions which breaks our internal check_discrete_distribution test.

I'm not familiar enough with this particular distribution to provide much guidance but there are a couple options:

  1. simply treat the distribution as continuous, allowing real value inputs for the "probability mass functions"
  2. Create a special case test for negative binomial (not sure how often this case will arise in practice)
  3. Create a special case distribution type

Option 1 seems a vastly superior choice but possibly semantically wrong (although also possibly to the point of pedantry)

@dsaxton
Copy link
Contributor

dsaxton commented Aug 27, 2021

Hi @boxtown, what was the original reasoning behind making r real-valued (i.e., what were the use cases)? I had only ever seen r interpreted as a count. I'm assuming a similar treatment isn't given to say n for the ordinary binomial distribution?

@WarrenWeckesser
Copy link
Contributor

statrs is still version 0.x, so wouldn't it be acceptable to change the type of r to an integer? I agree with @dsaxton; every reference that I've seen refers to the negative binomial distribution as a discrete distribution, so allowing that parameter to be continuous is unusual.

If it turns out later that there is sufficient demand for a continuous version, then perhaps a new distribution could be added then, with a name such as ContinuousNegativeBinomial, that makes it explicit that it is not the traditional discrete distribution. This would avoid the need to shoehorn an inherently discrete distribution into a continuous API, or vice versa.

@WarrenWeckesser
Copy link
Contributor

On the other hand, r is just a parameter of the distribution; whether that parameter is float or integer shouldn't change the "discreteness" of the distribution.

However, this causes the CDF to behave continuously rather than like in other discrete distributions which breaks our internal check_discrete_distribution test.

Could you explain what goes wrong? How does r being a float make the CDF "behave continuously"? The two tests at the end of negative_binomial.rs that use check_discrete_distribution (currently commented out) pass when I uncomment them. (I'll experiment further, but some details on the nature of the problem might be helpful.)

WarrenWeckesser added a commit to WarrenWeckesser/statrs that referenced this issue Feb 2, 2022
* Uncomment the check_discrete_distribution tests.
* Expand the documentation--explain the meaning of `r` and `p`.
* Note that `r` can be real.

Closes statrs-devgh-115.
WarrenWeckesser added a commit to WarrenWeckesser/statrs that referenced this issue Feb 2, 2022
* Uncomment the check_discrete_distribution tests.
* Expand the documentation--explain the meaning of `r` and `p`.
* Note that `r` can be real.

Closes statrs-devgh-115.
boxtown added a commit that referenced this issue Aug 29, 2022
* Uncomment the check_discrete_distribution tests.
* Expand the documentation--explain the meaning of `r` and `p`.
* Note that `r` can be real.

Closes gh-115.

Co-authored-by: Michael Ma <michael60612@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants