Update blackbox likelihood example by ricardoV94 · Pull Request #631 · pymc-devs/pymc-examples (original) (raw)

View / edit / reply to this conversation on ReviewNB

jessegrabowski commented on 2024-02-02T22:53:25Z
----------------------------------------------------------------

One problem I have with this framing is that it blurs a bit the purpose of this tutorial. Are we learning how to implement a likelihood (this is what the name suggests), or are we learning how to wrap arbitrary Pytensor Ops, a using the specific example of a likelihood?

I would prefer the framing to of the introduction to be very squarely on automatic logp inference, CustomDistand where you wouldn't want to use that. For example, when you know the logp of a model but not the generative process.

I'd like to see a separate tutorial called "Pytensor for PyMC Users" that covers how to wrap arbitrary python code for Pytensor use, then have this one focus more explicitly on the difference between pm.CustomDist and pm.Potential.

To help most users, I think this tutorial should be a flowchart like: Can you write down the generative process? --> CustomDist(dist=). Can you write down the logp --> Customdist(logp=) or pm.Potential.

ricardoV94 commented on 2024-02-14T11:20:41Z
----------------------------------------------------------------

I think we need a proper notebook on CustomDist with dist behavior, probably a core notebook, not even pymc-examples.

This tutorial has always been about a black-box likelihood, original a complicated cython function, that got removed in the last hackathon because it was too complicated for devs to maintain. Now we have this baby numpy alternative here, which I think it's fine.

The different between a blackbox Likelihood/model and individual Ops is thin. I don't want to delve into that here because it requires more PyTensor exposition. I just wanted to make this NB more useful than it was before not to change the pedagogy.

ricardoV94 commented on 2024-02-14T11:35:14Z
----------------------------------------------------------------

May be good to emphasize that we could black box the model part only, and still use a pm.Normal likelihood, or we can black-box both the model and likelihood (and even priors). The way the original example was written already lends itself to it, by separating my_model and my_loglike