-
-
Notifications
You must be signed in to change notification settings - Fork 407
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
Implement from_numpyro #811
Conversation
Maybe you could rename the variables that are common between stan and pymc3 so when we get more diagnostic plots we can have good defaults. Also, you might add original names to attrs? Maybe also have both tree depth and num_steps? |
Good idea! Thanks @ahartikainen , I'll make corresponding changes. |
Wow this is great. I accidentally created a mild conflict when I merged in another docs pr but it should be a straightforward fix :) Let me know if you need any help |
One question I do have, Are you planning on fixing the todos in this pr or in a later pr? |
Hi @canyon289 , currently, supporting these TODOs (extracting log_likelihood and observed_rvs) is not possible with NumPyro MCMC object (because we don't store model's args, kwargs). Though it is an easy fix, we have to wait for the next NumPyro release to solve these TODOs in arviz. I think I should mark this PR as WIP and wait for that release. :) |
If you want we could make Github issues to track the todos and merge this in since there's good functionality included already! It also reduces your chance of further merge conflicts. Let me know what you think :) |
Hi @canyon289 , that sounds a good plan to me because these TODOs are just for additional features (the API will not change). It has taken us a few months just for the last release but we hope that from now on, we will have more frequent release cycles. ^^ |
Awesome!! Merging |
This PR addresses #810.
I marked some TODOs for future PRs (when NumPyro MCMC object stores model arguments/keywords for the purpose of extracting log_likelihoods, observed_data). Currently, users need to use numpyro.handlers, which is not handy, to obtain these informations.
~Currently, the names of sample stats in NumPyro are:
potential_energy
num_steps
(which can be converted to tree depth by using np.log2 function)accept_prob
mean_accept_prob
(mean acceptance probability until current iteration during warmup adaptation or sampling)diverging
adapt_state.step_size
adapt_state.inverse_mass_matrix
Do I need to rename those fields in
sample_stats_to_xarray
function to match arviz conventions?~Resolution: rename fields to match PyMC3 stat names.