-
Notifications
You must be signed in to change notification settings - Fork 36
Remove resume_from
and default_chain_type
#1061
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
Conversation
resume_from
resume_from
and default_chain_type
c0201f5
to
1742b9b
Compare
DynamicPPL.jl documentation for PR #1061 is available at: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## breaking #1061 +/- ##
============================================
- Coverage 82.39% 82.38% -0.01%
============================================
Files 42 42
Lines 3818 3816 -2
============================================
- Hits 3146 3144 -2
Misses 672 672 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
looks good
one tiny concern is whether this really closes #1049: I assume
https://github.com/TuringLang/DynamicPPL.jl/blob/c672e3adecf6e6409decd55bd1cdad90631c65ed/ext/DynamicPPLMCMCChainsExt.jl#L15C12-L15C37 does the right things for different kind of chains, but I thought it's worth mentioning.
Thanks @sunxd3! Yeah, that's fair. |
Docs are now live here https://turinglang.org/docs/usage/sampling-options/#saving-and-resuming-sampling |
@penelopeysm sorry for missing this -- looks good! thanks |
Removes
resume_from=chn
, replaces withinitial_state=loadstate(chn)
.Also removes
default_chain_type
. The reason why I want to do this is because it's one step towards removingSampler
. The replacement for this is to specify a default chain type in Turing rather than in DynamicPPL. I'll do that in a separate PR. (Notice that the default chain type is MCMCChains.Chains, but MCMCChains isn't a dep of DynamicPPL. So it just makes more sense to define this in Turing anyway.)Closes TuringLang/Turing.jl#2675
Closes TuringLang/Turing.jl#2171
Closes #1049