Refactor missing discrete dists

[ricardoV94]

This PR refactors all missing univariate discrete distributions.

1. I decided to refactor the Constant distribution after some debate on Slack as to whether this should be deprecated or not. Its direct use might be pretty problematic without a custom step algorithm, but could still be useful indirectly, such as in mixture distributions. I made the dtype of the RV be floatX to accept non-integer outputs (even though this distribution should be treated as a pmf, see Mixture should not allow mixing of discrete and continuous distributions #4511 for why this might matter)

2. I created separate RandomOps for the ZeroInflated* distributions for now. Similarly to Replace custom RandomVariable for WeibullBetaRV #4683, this should only be done temporarily until we have Mixture Distributions working on V4 (it seems such approach was already attempted in the past WIP: Convert zero-inflated distributions to Mixture subclasses #2246, WIP Implement ZeroInflatedPoisson as a subclass of Mixture #1459)

---

Depending on what your PR does, here are a few things you might want to address in the description:

• what are the (breaking) changes that this PR makes?
• important background, or details about the implementation
• are the changes—especially new features—covered by tests and docstrings?
• linting/style checks have been run
• consider adding/updating relevant example notebooks
• right before it's ready to merge, mention the PR in the RELEASE-NOTES.md

[ricardoV94]

There must be a less hacking way to get a filled array given c and size, that works as other random generators would. np.full(size, c) doesn't seem to cut it, because it works with shape and not with size.

[twiecki]

Isn't shape and size the same thing here?

[ricardoV94]

No because c could be a vector and size could be None for instance

[ricardoV94]

Okay, actually I think the only problem was size=None. I added a check for that condition...

[twiecki]

This looks great!

[twiecki]

Thanks for taking this on @ricardoV94. Separately, when I tried V4 it seems like other continuous distributions are also missing, like StudentT?

[ricardoV94]

Yes they are missing.

Only distributions that have a rv_op at the top have been refactored. StudentT hasn't yet.

I won't have more time this week, so if someone wants to open a PR for the missing continuous distributions it would be great. The logic should be pretty similar to what is done in this PR with the custom RandomOps

[brandonwillard]

Looks good, but we really shouldn't rely on Distribution.logp functions, since those aren't officially part of the Distribution API—they're just a means of conveniently specifying dispatches.

[ricardoV94]

Looks good, but we really shouldn't rely on Distribution.logp functions, since those aren't officially part of the Distribution API—they're just a means of conveniently specifying dispatches.

@brandonwillard I am not sure what you have in mind. This looks wrong, right?

# at.log(psi) + Poisson.logp(value, theta)
at.log(psi) + _logp(poisson, value, {value: value}, theta)

I also tried this, but it fails following a change_rv_size operation:

at.log(psi) + logpt(Poisson.dist(theta), value)

[brandonwillard]

@brandonwillard I am not sure what you have in mind. This looks wrong, right?

# at.log(psi) + Poisson.logp(value, theta),
at.log(psi) + _logp(poisson, value, {value: value}, theta),

No, that looks like the correct approach.

[ricardoV94]

Could you give some intuition why this is better than the original approach?

[brandonwillard]

Could you give some intuition why this is better than the original approach?

As I stated in my original review comments, these Distribution.logp functions are not a part of the Distribution API and they are neither designed nor intended to be used in this way. Their availability is questionable and subject to arbitrary change, so we don't want to use them outside of their intended purpose.

More importantly, we do not want to rely on the Distribution types in general, and especially not for log-likelihoods; they're currently just a legacy transition artifact. This is clearly demonstrated by the lack of Distribution in the log-likelihood code itself.

Simply put, this approach unnecessarily adds Distributions into the log-likelihood framework.

If this sort of use case needs to be simplified, it can be done in better ways that are based only on the underlying dispatch.

[twiecki]

🥳