Skip to content

Conversation

@devmotion
Copy link
Member

This PR generalizes LocationScale to discrete distributions. This change is motivated by the example in TuringLang/MCMCChains.jl#263: the distribution of the statistic of interest is a PoissonBinomial distribution whose support is scaled with a constant factor. It seems natural to reuse the existing implementation of PoissonBinomial (which potentially can be more efficient than a regular DiscreteNonParametric distribution, as explained in #1285) instead of implementing a custom distribution for this use case.

When I extended the tests, I noticed the surprising defaults for (log)(c)cdf for discrete distributions which lead to bugs of log(c)cdf for DiscreteNonParametric (and potentially other discrete distributions). I fixed the issue for DiscreteNonParametric in this PR but maybe the bugfix should be extracted to a separate PR. In general, I think it would be reasonable to remove the defaults and make (log)(c)cdf consistent with (log)pdf: each distribution has to decide separately, for which inputs the functions are implemented and in particular if real numbers are rounded to integers.

I moved locationscale.jl to src/univariate/ in the first commit, so for reviewing it might be helpful to inspect the first and the other commits separately.

@devmotion
Copy link
Member Author

This generalization would be useful for the example mentioned above (otherwise I would have to copy and slightly modify the implementation of PoissonBinomial) but what is the general opinion here? I guess at least the problem with (log)(c)cdf should be fixed.

@devmotion
Copy link
Member Author

Bump 😃

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.

2 participants