-
-
Notifications
You must be signed in to change notification settings - Fork 9
Added from_numpyro_svi converter
#95
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #95 +/- ##
===========================================
- Coverage 72.34% 50.23% -22.11%
===========================================
Files 19 19
Lines 1837 1879 +42
===========================================
- Hits 1329 944 -385
- Misses 508 935 +427 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
from_numpyro_svi converterfrom_numpyro_svi converter
|
I added an MVP implementation of converting numpyro SVI results to ArviZ. There are two things in the PR that I could use feedback on: 1. the
|
from_numpyro_svi converterfrom_numpyro_svi converter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry about the extremely slow review time. I should be able to review way more quickly going forward. Thanks so much for the great PR with all CI green even ❤️
I agree with most decisions regarding if branching, inputs and tests (haven't really used VI extensively so not the most indicate person to weigh in though).
No worries at all, thanks for the review! I'm going to take a look at that Nested Sampler PR you linked as well and try to think about how the patterns I choose here could generalize |
|
@juanitorduz if you have time, inviting you to take a look and give feedback! I know you use numpyro, SVI, and ArviZ quite a bit |
|
Wow! This looks great! I always used the from_dict approach by hand 😅 |
updated typing for docstub fixed test_inferred_dims_univariate improved load_cached_models fixed svi docstring and SVIWrapper changed numpyro svi signature fixed from_numpyro_svi docstring added custom guide support and test added test support for custom guides
Co-authored-by: Tomás Capretto <[email protected]>
6971e4e to
4b0e1bf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a small comment about the notebook but otherwise I think it is ready to merge. We can then continue iterating over the sample_dims workflow
Background
Closes #2453 and #2452
Adds support for converting numpyro svi conversion to ArviZ. Also includes support for inferring dims automatically from the model. SVI uses sample dims of ("samples",) instead of ("chain", "draw").
A conversion guide for numpyro was also added, covering the following cases
📚 Documentation preview 📚: https://arviz-base--95.org.readthedocs.build/en/95/