let sampling return a vector of ParamsWithStats - #523
Conversation
Each sampler now only implements transition_params_and_stats, so the chain extensions stop duplicating the same unpacking per output format. Drops four extensions and makes MCMCChains handle its own statistic flattening.
chain_type = Vector{ParamsWithStats} returns one entry per draw, keyed by
VarName so array-valued variables stay whole. Needs no extension since
AbstractMCMC is a hard dependency. The callback constructor now uses the same
VarName-keyed container instead of stringified symbols.
AbstractMCMC.from_samples takes a draws matrix, so a single run converts with reshape(draws, :, 1).
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #523 +/- ##
==========================================
+ Coverage 87.79% 87.81% +0.02%
==========================================
Files 34 32 -2
Lines 3679 3645 -34
==========================================
- Hits 3230 3201 -29
+ Misses 449 444 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
JuliaBUGS.jl documentation for PR #523 is available at: |
A multivariate slice sampler reports num_proposals per coordinate, so converting those draws hit an unconvertible array element.
transition_params_and_stats now unpacks one transition into (params, stats), so a statistic a draw never reported stays absent rather than being padded with NaN. Deletes the slice sampler's padding, lets Chains take the union of keys and fill the gaps itself, and fixes MALA, whose GradientTransition the vector-shaped extractor could not service. The same method now drives the ParamsWithStats a callback sees, so HMC, MH and slice callbacks report VarName-keyed parameters in the model's space instead of falling through to unnamed theta[i] in the sampler's space.
…n non-numeric ones Gibbs and IndependentMH chains had an empty internals section and no log density at all. The evaluation that reconstructs each draw already computes it, so lp now costs nothing and every output format carries it, which is what makes the callback and the sampling output agree. Also: only numeric arrays expand into scalar Chains columns, so a sampler reporting anything else no longer breaks the conversion; array statistics are copied like array parameters; environment recovery dispatches on the sampler rather than guessing from the transition's shape; and from_samples(VNChain) gained the start/thin that the Chains version already had.
804a854 to
f9e23dc
Compare
Benchmark ResultsJulia 1.11.9 on INTEL(R) XEON(R) PLATINUM 8573C (Linux) Ratio = JuliaBUGS/Stan (lower is better for JuliaBUGS)
Note: Stan benchmarks use hand-optimized Stan models, not direct BUGS translations. Comparison is illustrative only. |
|
I forgot that it was already discussed to stay consistent with DynamicPPL -- approach here is not consistent so let me make the changes before asking for review again. |
|
#517 and TuringLang/AbstractPPL.jl#178, both closed. TuringLang/AbstractMCMC.jl#212 landed, so the wrapper is there. The container is not. this PR keeps Should I wait for TuringLang/AbstractPPL.jl#178, or keep this container and swap later? Making @sunxd3 Can you help me understand things here like in what direction do we want to go here? |
1 participant
Adds
chain_type = Vector{ParamsWithStats}: one entry per draw, params keyed byVarNameso array variables stay whole, stats as the sampler reported them.AbstractMCMC.from_samples(Chains, reshape(draws, :, 1))and the same withVNChainconvert them.Samplers now implement one method,
transition_params_and_stats(model, sampler, transition), and all three output formats are built from it.Other changes, breaking ones marked:
VarNamekeys in the model's space for every sampler; with HMC, MH and slice they used to give unnamedθ[i]in unconstrained space (breaking:pws.params.mu->pws.params[@varname(mu)])lp;internalswas empty before (breaking: one extra column)VNChainit ismissing, notNaN)gen_chainstakes oneNamedTupleper draw, not a names list plus a values table (breaking)param_1One question. I used-- #523 (comment)AbstractMCMC.ParamsWithStatsdirectly. DynamicPPL instead defines its ownParamsWithStatsand kept the AbstractMCMC one for callbacks only. Which do we want here?