-
Notifications
You must be signed in to change notification settings - Fork 33
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
allow edge case of negative binomial with 0 mean #366
Conversation
I think stan sees a reason (for whatever reason): "Error : Exception: Exception: neg_binomial_2_rng: Location parameter is 0, but must be > 0! (in 'functions/observation_model.stan' at line 117; included from 'model_estimate_infections' at line 7)" |
Exactly - which will no longer occur once this is merged. |
Sorry, my point was why does it do this and not follow the behaviour of R. |
f99c9dd
to
0978265
Compare
This is how benchmark results would change (along with a 95% confidence interval in relative change) if 7cc0d62 is merged into develop:
|
This is how benchmark results would change (along with a 95% confidence interval in relative change) if 01246d8 is merged into develop:
|
5fbdaf2
to
600227f
Compare
This is how benchmark results would change (along with a 95% confidence interval in relative change) if f6a0334 is merged into develop:
|
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 looks good I think. Just needs a news file entry before it can be merged..
This is how benchmark results would change (along with a 95% confidence interval in relative change) if 11c6fbc is merged into main:
|
allow edge case of negative binomial with 0 mean
This is how R handles it and I see no reason not to support this here instead of throwing an error