-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add step
dimension in sampler stats for compound steps
#4602
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
Comments
|
I just rediscovered this bug too. with pm.Model():
pm.Normal("a")
pm.Normal("b")
pm.sample(step=pm.Metropolis()) +1 for treating it as a dimension. But I don't think it will be a required dimension, because some stats like I'm not sure how |
An alternative to a Note that Lines 226 to 234 in 98cc942
With this example I don't think we can easily introduce a with pm.Model():
pm.Poisson("a", mu=1)
pm.Poisson("b", mu=2)
pm.Normal("c")
idata = pm.sample()
# CompoundStep
# >CompoundStep
# >>Metropolis: [a]
# >>Metropolis: [b]
# >NUTS: [c] |
🐛 : The ArviZ converter incorrectly drops the with pm.Model() as pmodel:
pm.Normal("a")
pm.Uniform("b")
pm.Uniform("c")
idata = pm.sample(step=pm.Metropolis(), chains=1)
idata.sample_stats.dims["chain"] == 1 # False
idata.sample_stats.dims["draw"] == 1000 # False
idata.sample_stats.dims["accepted_dim_0"] # KeyError |
what are the options on this? having different dimensions for different variables within the same dataset is not a problem. i.e. lp having only chain and draw but accept having chain, draw, sampler is no different from having chain, draw, accept_dim_0. What would be problematic for the coordinate values would be if there was the possibility of "combinatorics" with sample stats. i.e. out of 4 steppers, 3 use accept, 3 (but not the same) use accepted and only 2 use scaling. |
In the PyMC world we should only stack sampler stats coming from the same type of step method. Otherwise we could end up trying to stack two stats of the same name, but with different shapes/dtypes. Generally one could also argue to not stack them at all: They are diagnostics of separate samplers, and will be investigated separately. If we want to add meaningful dims, we should be able to specify them ahead of time: Step methods should report not only the name and dtype, but also the shape and optionally the dims. We should still fix that bug where the dims are mixed up in conversion from |
When using a compound step, the acceptance rate of each step function is reported separately. It would be nice to check for this at some point and pass the extra coordinate and dims to ArviZ via
idata_kwargs
so users can then useidata.sample_stats.accept.sel(step="Metropolis")
Caveats:
This came up in: pymc-devs/pymc-examples#94
The text was updated successfully, but these errors were encountered: