pymc icon indicating copy to clipboard operation
pymc copied to clipboard

Latex representation for SymbolicDistributions

Open larryshamalama opened this issue 3 years ago • 8 comments

Closes #5616.

This PR adds Latex presentation to SymbolicDistributions which does not construct rv_ops like regular Distributions, hence why the Latex representation is currently broken.

I would like some advice on how to carry this out. Dispatching was recommended, but the initial class which was called does not seem to be encoded in TensorVariables for SymbolicDistributions. I currently set it up so that a new str_for_symbolic_dist function would be called for SymbolicDistributions for _print_names which can be automatically initialized for such distributions (except for zero-inflated ones because they are created via pm.Mixture). Here are the internal changes that I came up with, but I'm happy to hear discussion about these points:

  • cls is now passed in _zero_inflated_mixture so they don't come out as "Mixture"
  • Akin to str_for_dist, str_for_symbolic_dist is created in printing.py in which pattern matching can be used to determine how to print the corresponding distribution. I have yet to address this and perhaps this is where dispatching would be useful?
  • _print_name attributes initialized for SymbolicDistributions

Some printing references that have been provided: AePPL printing and symbolic-pymc printing. Both of which use dispatching, but only the latter uses singledispatch.

larryshamalama avatar May 23 '22 18:05 larryshamalama

Codecov Report

Merging #5793 (60c3407) into main (7b239bb) will increase coverage by 0.27%. The diff coverage is 74.51%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5793      +/-   ##
==========================================
+ Coverage   89.36%   89.64%   +0.27%     
==========================================
  Files          74       73       -1     
  Lines       13757    13257     -500     
==========================================
- Hits        12294    11884     -410     
+ Misses       1463     1373      -90     
Impacted Files Coverage Δ
pymc/variational/__init__.py 100.00% <ø> (ø)
pymc/printing.py 60.00% <9.80%> (-26.14%) :arrow_down:
pymc/variational/approximations.py 86.51% <62.50%> (+20.44%) :arrow_up:
pymc/distributions/timeseries.py 78.64% <90.47%> (+1.17%) :arrow_up:
pymc/aesaraf.py 91.95% <100.00%> (+0.06%) :arrow_up:
pymc/distributions/discrete.py 99.73% <100.00%> (+<0.01%) :arrow_up:
pymc/distributions/distribution.py 90.94% <100.00%> (+0.11%) :arrow_up:
pymc/distributions/logprob.py 97.65% <100.00%> (+0.19%) :arrow_up:
pymc/distributions/mixture.py 95.72% <100.00%> (-1.04%) :arrow_down:
pymc/sampling.py 82.52% <100.00%> (-6.19%) :arrow_down:
... and 15 more

codecov[bot] avatar May 23 '22 19:05 codecov[bot]

Notes for myself for tomorrow or Wednesday:

  • How to deal with TensorConstants or random variable constants (like in the ZeroInflatedPoisson)
  • Single component mixture representation

And of course adding aforementioned yummy tests

larryshamalama avatar May 30 '22 18:05 larryshamalama

Need to check:

  • Censored distributions
  • Mixtures with non-constant mixing probabilities
  • Where UnmeasurableConstantRV comes from

larryshamalama avatar Jun 01 '22 15:06 larryshamalama

Mixing weights for mixture components needs to be discussed because it would hard to delineate them from nonzero_p in zero-inflated distributions. Also, in [p, 1 - p], 1 - p appears as an Elemwise.

larryshamalama avatar Jun 01 '22 21:06 larryshamalama

*Note: I just marked as ready for review, but it is not ready to be merged. It is missing tests and how to properly address mixture weights.

larryshamalama avatar Jun 01 '22 21:06 larryshamalama

@twiecki this needs to be a blocker for V4, otherwise you get errors when creating models with Distributions in an interactive environment like jupyter or ipython

ricardoV94 avatar Jun 03 '22 10:06 ricardoV94

@larryshamalama @ricardoV94 how much work is left on this one? Can we include it in the 4.0.2 milestone?

michaelosthege avatar Jul 01 '22 11:07 michaelosthege

To do it well, I think that there is still quite some work to be done. We wanted to do it the hackish way in preparation for v4 but we temporarily have this even more hackish PR merged. I just saw that you added this to the v4.2.0 milestone and I think that's appropriate since I'd like to work on this slowly.

larryshamalama avatar Jul 02 '22 11:07 larryshamalama

Closing this in favor of #6072

ricardoV94 avatar Aug 29 '22 09:08 ricardoV94