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 for from_pyro with multiple chains #463

Merged
merged 4 commits into from
Dec 27, 2018

Conversation

jeffpollock9
Copy link
Contributor

As per #462 - we should only expand the dimension of the samples if the number of chains is 1. Please note I have not tested this for pyro versions less than 0.3.

For the example in the #462, this now correctly prints:

>>> az.summary(data)
          mean    sd  mc error  hpd 3%  hpd 97%  eff_n  r_hat
alpha    45.46  0.08       0.0   45.30    45.59  142.0   1.00
beta[0]   1.25  0.08       0.0    1.11     1.41  164.0   1.01
beta[1]   2.21  0.09       0.0    2.05     2.40  141.0   1.00
sigma     0.79  0.06       0.0    0.69     0.89   77.0   1.00

when running with 2 chains.

@ahartikainen
Copy link
Contributor

ahartikainen commented Dec 26, 2018

.num_chains is apparently missing from the version used in travis.

I think you need to send posterior or num_chains to function. self is referring to class, it is not defined outside the class.

@ahartikainen
Copy link
Contributor

I think you can skip the function and just duplicate your if-else structure, this way everything should work.

@jeffpollock9
Copy link
Contributor Author

@ahartikainen thanks. I think num_chains is missing in pyro < 0.3 since they didn't support running multiple chains in parallel at that point - so the untested changes I made were dumb (serves me right for not properly testing). That is at least as far as I can tell looking at e.g. https://github.com/uber/pyro/blob/0.3.0-release/pyro/infer/mcmc/mcmc.py#L103

Hoping it is ok now.

@ahartikainen
Copy link
Contributor

LGTM

@ahartikainen ahartikainen merged commit 7ccd004 into arviz-devs:master Dec 27, 2018
@jeffpollock9 jeffpollock9 deleted the pyro-multiple-chains-fix branch January 2, 2019 16:12
@jeffpollock9 jeffpollock9 restored the pyro-multiple-chains-fix branch January 2, 2019 16:28
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.

2 participants