-
-
Notifications
You must be signed in to change notification settings - Fork 190
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
Added the Dirichlet-Multinomial distribution (issue 54) #2979
Added the Dirichlet-Multinomial distribution (issue 54) #2979
Conversation
Thanks! Can you also add a test in |
Thanks @SteveBronder! I wrote the AD test. But before I push this, do you know how to fix this continuous integration issue I get? I think it might be because I named my fork "stan_math" instead of "math" |
Yes the name looks to be an issue. We can fix it on our side but it might just be easier to change the name of your fork |
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.
Thanks for the contribution! A couple of minor requests to code style and files.
I've also updated the lpmf
function to take advantage of our/Eigen's vectorised functions and added analytical gradients. Let me know if there's anything in those changes that you disagree with or similar.
Thanks!
Thanks @andrjohns for the improvements. I've added a |
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.
LGTM! Thanks for adding this!
Summary
This PR adds the Dirichlet-Multinomial distribution to the Stan math library. The Dirichlet-Multinomial (DirMult) distribution generalizes the Beta-Binomial distribution with more than two categories. It can also be seen as an over-dispersed multinomial distribution. It is implemented in other popular frameworks such as Pyro and the Python package scipy. In issue 54 (first created in 2014) it is suggested to add this distribution.
I largely based my implementation on the existing multinomial distribution. As such the DirMult distribution is currently not vectorized, as I could not find an example of a vectorized multivariate discrete distribution in the current code base. However, I would be interested in implementing vectorization in a future PR. I have added clear Doxygen documentation for the lpmf, rng (and log) functions.
I added 4 files for the log-PMF, the log function, the PRNG, a number of tests, and I updated the prob.hpp header. I have also prepared PRs for stanc3 and the docs, and tested the new native distribution in action with a Stan model using cmdstanpy.
Tests
I have included 8 unit tests.
Side Effects
There should not be any side effects.
Release notes
Added the Dirichlet-Multinomial distribution to the Stan Math library (dirichlet_multinomial_lpmf, dirichlet_multinomial_log, and dirichlet_multinomial_rng).
Checklist
By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses:
- Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
- Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)
[x ] the basic tests are passing
./runTests.py test/unit
)make test-headers
)make test-math-dependencies
)make doxygen
)make cpplint
)[x ] the code is written in idiomatic C++ and changes are documented in the doxygen
[x ] the new changes are tested