Replace zero inflated distributions with mixtures

[ricardoV94]

Required the following extra changes:

• Implement logcdf for Mixtures
• Allow Constant distribution to have a varying dtype

[codecov]

Codecov Report

Merging #5584 (68bb77a) into main (86f8d98) will decrease coverage by 0.04%.
The diff coverage is 96.49%.

@@            Coverage Diff             @@
##             main    #5584      +/-   ##
==========================================
- Coverage   87.64%   87.59%   -0.05%     
==========================================
  Files          76       76              
  Lines       13775    13732      -43     
==========================================
- Hits        12073    12029      -44     
- Misses       1702     1703       +1     

Impacted Files	Coverage Δ
pymc/distributions/mixture.py	93.14% <91.30%> (-0.03%)	⬇️
pymc/distributions/discrete.py	99.72% <100.00%> (-0.04%)	⬇️

[twiecki]

Nice refactor.

[larryshamalama]

Overall looks great! Left some questions so I can better understand myself 😅 (and maybe for others reading this PR!)

An overarching question that I have is what benefits do we get from positing a Mixture-type rv_op for zero-inflated distributions? My understanding was that Mixture distributions inherit from SymbolicDistribution as a way to initialize a different rv_op depending on how many and what comp_dists are provided. However, for zero-inflated distributions, there will always be "two" components: the zero Constant distribution and Poisson/binomial/negative binomial. Perhaps the answer is simple but I was just wondering...

[larryshamalama]

Does nonzero_dist here have to be Discrete?

[ricardoV94]

I think your question might be related to #4511

Still need to make sure we are not combining discrete and continuous distributions in mixtures. In the cases here they are both supposed to be discrete yes

[ricardoV94]

Did you have another thing in mind?

[larryshamalama]

This is exactly what I had in mind! I was wondering if it would help to have a check that nonzero_dist is discrete, but, as you mentioned above, it may not be necessary since _zero_inflated_mixture is not a user-facing function.

[ricardoV94]

Yes, and I want to add an explicit check in the Mixture class in a follow up PR

[ricardoV94]

An overarching question that I have is what benefits do we get from positing a Mixture-type rv_op for zero-inflated distributions

The biggest advantage is less code for us to maintain. As Thomas hinted above this is just a refactor.

Reusing a building block also means any bug fixes there propagate everywhere quickly. For instance, the old RandomVariable Ops actually had a slight bug in that they did not broadcast psi with the other parameters when size was not specified. So pm.ZIP.dist(0.5, [200, 300]) Could never draw mixed zeros and non zeros, even though it should have.

Instead of having to fix this everywhere, it's now enough to do it (and test it) in one place as I did here: 78f1786

[twiecki]

What about the integrated versions? I guess we get those automatically once it's added to aeppl?

[ricardoV94]

What about the integrated versions? I guess we get those automatically once it's added to aeppl?

What do you mean integrated?

[twiecki]

What do you mean integrated?

Upon thinking about it I don't think it makes sense here.

[larryshamalama]

The biggest advantage is less code for us to maintain. As Thomas hinted above this is just a refactor.

Reusing a building block also means any bug fixes there propagate everywhere quickly. For instance, the old RandomVariable Ops actually had a slight bug in that they did not broadcast psi with the other parameters when size was not specified. So pm.ZIP.dist(0.5, [200, 300]) Could never draw mixed zeros and non zeros, even though it should have.

Instead of having to fix this everywhere, it's now enough to do it (and test it) in one place as I did here: 78f1786

Okay I see, thanks for the clarification! Makes a lot of sense