-
Notifications
You must be signed in to change notification settings - Fork 11
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
add truncated negative binomial #25
base: master
Are you sure you want to change the base?
Conversation
double u, pa, pb; | ||
pa = R::pnbinom(a, size, prob, true, false); | ||
pb = R::pnbinom(b, size, prob, true, false); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part repeats in all methods, so probably could be extracted as a separate method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@twolodzko Please elaborate. Are you suggesting something like below?
double u, pa, pb;
tie(pa, pb) = pnbinom_ends(a, b, size, prob);
We'd need a second pnbinom_ends_mu
. Maybe you can propose a better name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean that calling R::pnbinom_mu
and R::pnbinom
with the same parameters happens several times in the code so they can be extracted as separate functions.
Could you rebase with master? I made some minor fixes. |
Besides the refactor, I don't have other comments. |
This adds a truncated negative binomial distribution, including support for both
prob
andmu
parameterizations. Tests are included, covering a similar regimen as truncated Poisson.Description is updated to minor bump the version, include the new distribution, and add authorship.