support warmup groups in from_pymc3 #1171
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1171 +/- ##
=======================================
Coverage 93.11% 93.11%
=======================================
Files 94 94
Lines 9322 9339 +17
=======================================
+ Hits 8680 8696 +16
- Misses 642 643 +1
Continue to review full report at Codecov.
|
|
looks like pylint doesn't want us to access |
|
Ready for review or even merge Also related to #1083, this starts storing number of tuning steps in inference data as attrs of posterior and sample stats (both samples and warmup). |
| assert idata.posterior.sizes["chain"] == 2 | ||
| assert idata.posterior.sizes["draw"] == 200 | ||
| if save_warmup: | ||
| # pylint: disable=protected-access | ||
| assert idata._warmup_posterior.dims["chain"] == 2 | ||
| assert idata._warmup_posterior.dims["draw"] == 100 |
There was a problem hiding this comment.
- Just out of curiosity: why is the attribute to access different for
posterior(sizes) and_warmup_posterior(dims), whereas you want to see the number of chains and draws in both cases? - Also, does it mean we keep the
_beforewarmup?
There was a problem hiding this comment.
_ should stay before warmup. it is not a mcmc draw (in hmc; maybe) so it is only for advanced users
There was a problem hiding this comment.
Regarding first question, both attributes do the same, the test Michael sent had sizes and I used dims to make it more xarray-y but missed half of them 🤷
There was a problem hiding this comment.
@ahartikainen how opposed would you be to removing the _ before warmup? Even if only for advanced users it is still for users, the default would still be data.save_warmup = false so unless they change it they would not have warmup groups, I feel like hiding them so much makes harder the live of advanced users without much gain for novices.
Co-authored-by: Michael Osthege <zufallsprinzip@hotmail.de>
|
I think it is ready to merge |
|
Agreed 👍
Le mer. 6 mai 2020 à 23:02, Oriol Abril <notifications@github.com> a écrit :
… I think it is ready to merge
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1171 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHIJMTAPLID3JKE3AEQH7X3RQHF53ANCNFSM4MY3TLBQ>
.
|
Description
Support warmup groups in
from_pymc3. It should already work, but I have not figured out the dependencies of the whole thing (it may only work on pymc3 master) nor added any test yet.Will fix #1146
Checklist