Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Predictive work with the SplitReparam reparameterizer [bugfix] #3388

Merged
merged 7 commits into from
Aug 4, 2024

Conversation

BenZickel
Copy link
Contributor

@BenZickel BenZickel commented Jul 26, 2024

Problem

Using pyro.infer.Predictive with a model that utilizes the pyro.infer.reparam.SplitReparam reparameterizer raises an error as pyro.infer.Predictive tries to sample from the model in order to determine site shapes.

Straightforward sampling of a model that utilizes the pyro.infer.reparam.SplitReparam reparameterizer is not possible as this reparameterizer introduces sites with the pyro.distributions.ImproperUniform distribution, which does not support sampling.

Solution

Wrap the model with the pyro.poutine.InitMessenger effect handler during the site shapes determination phase. This solves the problem as the pyro.poutine.InitMessenger effect handler assigns values to the pyro.distributions.ImproperUniform sites before they are sampled.

This is a specific feature of pyro.poutine.ReparamMessenger, which applies initialization by pyro.poutine.InitMessenger before sampling, even if it appears last in the messenger stack (see #2876).

Testing

The fix can be verified by running

pytest tests\infer\reparam\test_split.py::test_predictive

Copy link
Member

@fritzo fritzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clear PR description of this subtle fix!

@fritzo fritzo merged commit 6130da0 into pyro-ppl:dev Aug 4, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants