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

Fix the pyro samplers #288

Closed
wants to merge 1 commit into from
Closed

Fix the pyro samplers #288

wants to merge 1 commit into from

Conversation

michaeldeistler
Copy link
Contributor

@michaeldeistler michaeldeistler commented Aug 4, 2020

Fix for #286

Ok, I'm still pretty confused why this happens. It seems that, after the mcmc chain is run, .cleanup is called here which then calls ._reset(), which sets the init_params=None, see here.

However, after all this, transforms are performed on the samples. If transforms=None was specified, a transformation is automatically inferred, as explained here. However, this again sets up the kernel here, which requires init_params. It breaks because the init_params had been set to None.

A fix is to set transforms={}. With this, transforms are not automatically inferred and everything works fine.

--- Update ---
This is the exact commit that broke our code. After this change, if kernel is None, it still tries to run self.kernel.setup, which will break.

Should we report this as an issue to pyro?

@michaeldeistler michaeldeistler linked an issue Aug 4, 2020 that may be closed by this pull request
@alvorithm
Copy link
Contributor

I am not familiar with this part of the code and it is unlikely I can look at it before my holidays - sorry!

@michaeldeistler
Copy link
Contributor Author

michaeldeistler commented Aug 5, 2020

See pyro-ppl/pyro#2585

@michaeldeistler
Copy link
Contributor Author

I'm closing this. Pyro has fixed the bug: pyro-ppl/pyro#2585

As soon as they release v1.4.1, we can lift our ==1.3.1 pin and also close the issue #286

@jan-matthis jan-matthis mentioned this pull request Sep 23, 2020
@janfb janfb deleted the fix-pyro-samplers branch January 29, 2021 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pyro v1.4.0 compatibility
2 participants