-
Notifications
You must be signed in to change notification settings - Fork 36
Add to_chains and from_chains function
#1087
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
Benchmark Report for Commit 2fecc74Computer InformationBenchmark Results |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1087 +/- ##
==========================================
+ Coverage 81.06% 81.38% +0.32%
==========================================
Files 40 41 +1
Lines 3749 3798 +49
==========================================
+ Hits 3039 3091 +52
+ Misses 710 707 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
96df1a4 to
70c3dd9
Compare
70c3dd9 to
7049125
Compare
|
DynamicPPL.jl documentation for PR #1087 is available at: |
7b337bd to
833cbbf
Compare
Seems to work like a charm for Pathfinder! mlcolab/Pathfinder.jl#274 |
|
I wonder, for other packages implementing new |
So there are a couple of potential stages: Chain ----> Dict{VarName,Any} --[model]--> VarInfo.
|
to_chains functionto_chains and from_chains function
This PR adds a new struct
ParamsWithStatsand functionsto_chainsandfrom_chainswhich is mainly meant for developers of packages that share an interface with DynamicPPL.I would say that the main purpose of these function are to abstract away the inner details of chain construction so that this doesn't have to be duplicated everywhere. For example, there are at least four different places that feature the 'split-up-dicts-of-varnames' game for MCMCChains:
(1)
AbstractMCMC.bundle_sampleshttps://github.com/TuringLang/Turing.jl/blob/0eb8576c2c1f659aafdc1a22fc6396e0b1588a67/src/mcmc/Inference.jl#L311-L312(2)
DynamicPPL.predictDynamicPPL.jl/ext/DynamicPPLMCMCChainsExt.jl
Lines 167 to 188 in 1b159a6
(3) This DynamicPPL test utility
DynamicPPL.jl/test/test_util.jl
Lines 64 to 94 in 1b159a6
(4)
Pathfinder.pathfinderhttps://github.com/mlcolab/Pathfinder.jl/blob/6389f125197110ff35ccddc10ed682e4b9ff8c12/ext/PathfinderTuringExt.jl#L49Another benefit is that certain details, like the
varname_to_symbolDict that is stored with the chain, are implemented at the same level at which it's being used.The eagle-eyed will notice that
ParamsWithStatsis effectively the same asTuring.Inference.Transition, just without the logp terms explicitly bundled in.Furthermore,
to_chainsin the MCMCChainsExt is almost completely the same asbundle_samplesin Turing (although perhaps implemented in a slightly simpler way).I did it this way because I want Turing to be able to make use of this function. In an original draft I had
to_chainstake an array of VarInfo, and then perform the reevaluation. However, this makes it quite complicated to use this in the MCMC sampling bits of Turing.