From b1f21aa8f73f14c1933df1ba2834b9b6595eedd5 Mon Sep 17 00:00:00 2001 From: Penelope Yong Date: Mon, 8 Sep 2025 17:24:22 +0100 Subject: [PATCH 1/5] Remove `Sampler` and `initialstep` --- HISTORY.md | 5 +++ docs/src/api.md | 12 +++--- src/DynamicPPL.jl | 2 - src/sampler.jl | 95 ++--------------------------------------------- test/sampler.jl | 15 +++----- 5 files changed, 19 insertions(+), 110 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 90864508b..6a090e91c 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -42,6 +42,11 @@ The main change that this is likely to create is for those who are implementing The exact way in which this happens will be detailed in the Turing.jl changelog when a new release is made. Broadly speaking, though, `SamplingContext(MySampler())` will be removed so if your sampler needs custom behaviour with the tilde-pipeline you will likely have to define your own context. +### Removal of `DynamicPPL.Sampler` + +`DynamicPPL.Sampler` and `DynamicPPL.initialstep` have also been removed entirely. +If you were using these, you should unwrap your sampler and implement `AbstractMCMC.step` as usual for any other `AbstractMCMC.AbstractSampler`. + ### Simplification of the tilde-pipeline There are now only two functions in the tilde-pipeline that need to be overloaded to change the behaviour of tilde-statements, namely, `tilde_assume!!` and `tilde_observe!!`. diff --git a/docs/src/api.md b/docs/src/api.md index dcf4558bc..11ee0b3b2 100644 --- a/docs/src/api.md +++ b/docs/src/api.md @@ -504,23 +504,21 @@ DynamicPPL.AbstractInitStrategy DynamicPPL.init ``` -### Samplers +### Sampling -In DynamicPPL a generic sampler for inference is implemented. +`init_strategy(spl)` defines how parameters are to be initialised when performing MCMC sampling with `spl`. ```@docs -Sampler +DynamicPPL.init_strategy ``` -The default implementation of [`Sampler`](@ref) uses the following unexported functions. +`loadstate` is used for resuming sampling from a previous chain. ```@docs -DynamicPPL.initialstep DynamicPPL.loadstate -DynamicPPL.init_strategy ``` -Finally, to specify which varinfo type a [`Sampler`](@ref) should use for a given [`Model`](@ref), this is specified by [`DynamicPPL.default_varinfo`](@ref) and can thus be overloaded for each `model`-`sampler` combination. This can be useful in cases where one has explicit knowledge that one type of varinfo will be more performant for the given `model` and `sampler`. +Finally, to specify which varinfo type a sampler should use for a given [`Model`](@ref), this is specified by [`DynamicPPL.default_varinfo`](@ref) and can thus be overloaded for each `model`-`sampler` combination. This can be useful in cases where one has explicit knowledge that one type of varinfo will be more performant for the given `model` and `sampler`. ```@docs DynamicPPL.default_varinfo diff --git a/src/DynamicPPL.jl b/src/DynamicPPL.jl index b7631c293..523e00a4c 100644 --- a/src/DynamicPPL.jl +++ b/src/DynamicPPL.jl @@ -92,8 +92,6 @@ export AbstractVarInfo, getargnames, extract_priors, values_as_in_model, - # Samplers - Sampler, # LogDensityFunction LogDensityFunction, # Contexts diff --git a/src/sampler.jl b/src/sampler.jl index 4cc56d8ff..99858e24e 100644 --- a/src/sampler.jl +++ b/src/sampler.jl @@ -1,23 +1,3 @@ -# TODO(mhauru) Could we get rid of Sampler now that it's just a wrapper around `alg`? -# (Selector has been removed). -""" - Sampler{T} - -Generic sampler type for inference algorithms of type `T` in DynamicPPL. - -`Sampler` should implement the AbstractMCMC interface, and in particular -`AbstractMCMC.step`. A default implementation of the initial sampling step is -provided that supports resuming sampling from a previous state and setting initial -parameter values. It requires to overload [`loadstate`](@ref) and [`initialstep`](@ref) -for loading previous states and actually performing the initial sampling step, -respectively. Additionally, sometimes one might want to implement an [`init_strategy`](@ref) -that specifies how the initial parameter values are sampled if they are not provided. -By default, values are sampled from the prior. -""" -struct Sampler{T} <: AbstractSampler - alg::T -end - """ default_varinfo(rng, model, sampler) @@ -72,71 +52,6 @@ function _convert_initial_params(::AbstractVector) throw(ArgumentError(errmsg)) end -function AbstractMCMC.sample( - rng::Random.AbstractRNG, - model::Model, - sampler::Sampler, - N::Integer; - initial_params=init_strategy(sampler), - initial_state=nothing, - kwargs..., -) - return AbstractMCMC.mcmcsample( - rng, - model, - sampler, - N; - initial_params=_convert_initial_params(initial_params), - initial_state, - kwargs..., - ) -end - -function AbstractMCMC.sample( - rng::Random.AbstractRNG, - model::Model, - sampler::Sampler, - parallel::AbstractMCMC.AbstractMCMCEnsemble, - N::Integer, - nchains::Integer; - initial_params=fill(init_strategy(sampler), nchains), - initial_state=nothing, - kwargs..., -) - return AbstractMCMC.mcmcsample( - rng, - model, - sampler, - parallel, - N, - nchains; - initial_params=map(_convert_initial_params, initial_params), - initial_state, - kwargs..., - ) -end - -function AbstractMCMC.step( - rng::Random.AbstractRNG, - model::Model, - spl::Sampler; - initial_params::AbstractInitStrategy=init_strategy(spl), - kwargs..., -) - # Generate the default varinfo. Note that any parameters inside this varinfo - # will be immediately overwritten by the next call to `init!!`. - vi = default_varinfo(rng, model, spl) - - # Fill it with initial parameters. Note that, if `InitFromParams` is used, the - # parameters provided must be in unlinked space (when inserted into the - # varinfo, they will be adjusted to match the linking status of the - # varinfo). - _, vi = init!!(rng, model, vi, initial_params) - - # Call the actual function that does the first step. - return initialstep(rng, model, spl, vi; initial_params, kwargs...) -end - """ loadstate(chain::AbstractChains) @@ -144,13 +59,11 @@ Load sampler state from an `AbstractChains` object. This function should be over concrete Chains implementation. """ function loadstate end +loadstate(data) = data """ - initialstep(rng, model, sampler, varinfo; kwargs...) - -Perform the initial sampling step of the `sampler` for the `model`. + default_chain_type(sampler) -The `varinfo` contains the initial samples, which can be provided by the user or -sampled randomly. +Default type of the chain of posterior samples from `sampler`. """ -function initialstep end +default_chain_type(::AbstractSampler) = Any diff --git a/test/sampler.jl b/test/sampler.jl index 3fe7f2b07..9ed8d74d1 100644 --- a/test/sampler.jl +++ b/test/sampler.jl @@ -94,28 +94,23 @@ abstract type OnlyInitAlg end struct OnlyInitAlgDefault <: OnlyInitAlg end struct OnlyInitAlgUniform <: OnlyInitAlg end - function DynamicPPL.initialstep( - rng::Random.AbstractRNG, - model::Model, - ::Sampler{<:OnlyInitAlg}, - vi::AbstractVarInfo; - kwargs..., + function AbstractMCMC.step( + rng::Random.AbstractRNG, model::Model, ::OnlyInitAlg, kwargs... ) return vi, nothing end # initial samplers - DynamicPPL.init_strategy(::Sampler{OnlyInitAlgUniform}) = InitFromUniform() - @test DynamicPPL.init_strategy(Sampler(OnlyInitAlgDefault())) == InitFromPrior() + DynamicPPL.init_strategy(::OnlyInitAlgUniform) = UniformInit() + @test DynamicPPL.init_strategy(OnlyInitAlgDefault()) == PriorInit() - for alg in (OnlyInitAlgDefault(), OnlyInitAlgUniform()) + for sampler in (OnlyInitAlgDefault(), OnlyInitAlgUniform()) # model with one variable: initialization p = 0.2 @model function coinflip() p ~ Beta(1, 1) return 10 ~ Binomial(25, p) end model = coinflip() - sampler = Sampler(alg) lptrue = logpdf(Binomial(25, 0.2), 10) let inits = InitFromParams((; p=0.2)) chain = sample(model, sampler, 1; initial_params=inits, progress=false) From e5e98e12193dc2dfde170eb55d3ad34b601ac711 Mon Sep 17 00:00:00 2001 From: Penelope Yong Date: Mon, 13 Oct 2025 20:19:50 +0100 Subject: [PATCH 2/5] Actually just remove the entire file --- HISTORY.md | 10 +- docs/src/api.md | 20 +--- ext/DynamicPPLMCMCChainsExt.jl | 12 -- src/DynamicPPL.jl | 3 - src/sampler.jl | 69 ----------- test/runtests.jl | 1 - test/sampler.jl | 211 --------------------------------- 7 files changed, 9 insertions(+), 317 deletions(-) delete mode 100644 src/sampler.jl delete mode 100644 test/sampler.jl diff --git a/HISTORY.md b/HISTORY.md index 6a090e91c..2f9dabb2c 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -44,8 +44,14 @@ Broadly speaking, though, `SamplingContext(MySampler())` will be removed so if y ### Removal of `DynamicPPL.Sampler` -`DynamicPPL.Sampler` and `DynamicPPL.initialstep` have also been removed entirely. -If you were using these, you should unwrap your sampler and implement `AbstractMCMC.step` as usual for any other `AbstractMCMC.AbstractSampler`. +`DynamicPPL.Sampler` and **all associated interface functions** have also been removed entirely. +If you were using these, the corresponding replacements are: + + - `DynamicPPL.Sampler(S)`: just don't wrap `S`; but make sure `S` subtypes `AbstractMCMC.AbstractSampler` + - `DynamicPPL.initialstep`: directly implement `AbstractMCMC.step` and `AbstractMCMC.step_warmup` as per the AbstractMCMC interface + - `DynamicPPL.loadstate`: `Turing.loadstate` (will be introduced in the next version) + - `DynamicPPL.initialsampler`: `Turing.init_strategy` (will be introduced in the next version; note that this function must return an `AbstractInitStrategy`, see above for explanation) + - `DynamicPPL.default_varinfo`: `Turing.default_varinfo` (will be introduced in the next version) ### Simplification of the tilde-pipeline diff --git a/docs/src/api.md b/docs/src/api.md index 11ee0b3b2..bb67468d9 100644 --- a/docs/src/api.md +++ b/docs/src/api.md @@ -504,25 +504,7 @@ DynamicPPL.AbstractInitStrategy DynamicPPL.init ``` -### Sampling - -`init_strategy(spl)` defines how parameters are to be initialised when performing MCMC sampling with `spl`. - -```@docs -DynamicPPL.init_strategy -``` - -`loadstate` is used for resuming sampling from a previous chain. - -```@docs -DynamicPPL.loadstate -``` - -Finally, to specify which varinfo type a sampler should use for a given [`Model`](@ref), this is specified by [`DynamicPPL.default_varinfo`](@ref) and can thus be overloaded for each `model`-`sampler` combination. This can be useful in cases where one has explicit knowledge that one type of varinfo will be more performant for the given `model` and `sampler`. - -```@docs -DynamicPPL.default_varinfo -``` +### Choosing a suitable VarInfo There is also the _experimental_ [`DynamicPPL.Experimental.determine_suitable_varinfo`](@ref), which uses static checking via [JET.jl](https://github.com/aviatesk/JET.jl) to determine whether one should use [`DynamicPPL.typed_varinfo`](@ref) or [`DynamicPPL.untyped_varinfo`](@ref), depending on which supports the model: diff --git a/ext/DynamicPPLMCMCChainsExt.jl b/ext/DynamicPPLMCMCChainsExt.jl index 7886ad468..47ef8aca4 100644 --- a/ext/DynamicPPLMCMCChainsExt.jl +++ b/ext/DynamicPPLMCMCChainsExt.jl @@ -3,18 +3,6 @@ module DynamicPPLMCMCChainsExt using DynamicPPL: DynamicPPL, AbstractPPL using MCMCChains: MCMCChains -# Load state from a `Chains`: By convention, it is stored in `:samplerstate` metadata -function DynamicPPL.loadstate(chain::MCMCChains.Chains) - if !haskey(chain.info, :samplerstate) - throw( - ArgumentError( - "The chain object does not contain the final state of the sampler: Metadata `:samplerstate` missing.", - ), - ) - end - return chain.info[:samplerstate] -end - _has_varname_to_symbol(info::NamedTuple{names}) where {names} = :varname_to_symbol in names function DynamicPPL.supports_varname_indexing(chain::MCMCChains.Chains) diff --git a/src/DynamicPPL.jl b/src/DynamicPPL.jl index 523e00a4c..f5bd33d6d 100644 --- a/src/DynamicPPL.jl +++ b/src/DynamicPPL.jl @@ -126,8 +126,6 @@ export AbstractVarInfo, prefix, returned, to_submodel, - # Chain save/resume - loadstate, # Convenience macros @addlogprob!, value_iterator_from_chain, @@ -179,7 +177,6 @@ include("contexts/transformation.jl") include("contexts/prefix.jl") include("contexts/conditionfix.jl") # Must come after contexts/prefix.jl include("model.jl") -include("sampler.jl") include("varname.jl") include("distribution_wrappers.jl") include("submodel.jl") diff --git a/src/sampler.jl b/src/sampler.jl deleted file mode 100644 index 99858e24e..000000000 --- a/src/sampler.jl +++ /dev/null @@ -1,69 +0,0 @@ -""" - default_varinfo(rng, model, sampler) - -Return a default varinfo object for the given `model` and `sampler`. - -The default method for this returns a NTVarInfo (i.e. 'typed varinfo'). - -# Arguments -- `rng::Random.AbstractRNG`: Random number generator. -- `model::Model`: Model for which we want to create a varinfo object. -- `sampler::AbstractSampler`: Sampler which will make use of the varinfo object. - -# Returns -- `AbstractVarInfo`: Default varinfo object for the given `model` and `sampler`. -""" -function default_varinfo(rng::Random.AbstractRNG, model::Model, ::AbstractSampler) - # Note that in `AbstractMCMC.step`, the values in the varinfo returned here are - # immediately overwritten by a subsequent call to `init!!`. The reason why we - # _do_ create a varinfo with parameters here (as opposed to simply returning - # an empty `typed_varinfo(VarInfo())`) is to avoid issues where pushing to an empty - # typed VarInfo would fail. This can happen if two VarNames have different types - # but share the same symbol (e.g. `x.a` and `x.b`). - # TODO(mhauru) Fix push!! to work with arbitrary lens types, and then remove the arguments - # and return an empty VarInfo instead. - return typed_varinfo(VarInfo(rng, model)) -end - -""" - init_strategy(sampler::AbstractSampler) - -Define the initialisation strategy used for generating initial values when -sampling with `sampler`. Defaults to `InitFromPrior()`, but can be overridden. -""" -init_strategy(::AbstractSampler) = InitFromPrior() - -""" - _convert_initial_params(initial_params) - -Convert `initial_params` to an `AbstractInitStrategy` if it is not already one. -""" -_convert_initial_params(initial_params::AbstractInitStrategy) = initial_params -function _convert_initial_params(nt::NamedTuple) - @info "Using a NamedTuple for `initial_params` will be deprecated in a future release. Please use `InitFromParams(namedtuple)` instead." - return InitFromParams(nt) -end -function _convert_initial_params(d::AbstractDict{<:VarName}) - @info "Using a Dict for `initial_params` will be deprecated in a future release. Please use `InitFromParams(dict)` instead." - return InitFromParams(d) -end -function _convert_initial_params(::AbstractVector) - errmsg = "`initial_params` must be a `NamedTuple`, an `AbstractDict{<:VarName}`, or ideally an `AbstractInitStrategy`. Using a vector of parameters for `initial_params` is no longer supported. Please see https://turinglang.org/docs/usage/sampling-options/#specifying-initial-parameters for details on how to update your code." - throw(ArgumentError(errmsg)) -end - -""" - loadstate(chain::AbstractChains) - -Load sampler state from an `AbstractChains` object. This function should be overloaded by a -concrete Chains implementation. -""" -function loadstate end -loadstate(data) = data - -""" - default_chain_type(sampler) - -Default type of the chain of posterior samples from `sampler`. -""" -default_chain_type(::AbstractSampler) = Any diff --git a/test/runtests.jl b/test/runtests.jl index 2b92a023d..b6a3f7bf6 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -61,7 +61,6 @@ include("test_util.jl") include("varinfo.jl") include("simple_varinfo.jl") include("model.jl") - include("sampler.jl") include("distribution_wrappers.jl") include("logdensityfunction.jl") include("linking.jl") diff --git a/test/sampler.jl b/test/sampler.jl deleted file mode 100644 index 9ed8d74d1..000000000 --- a/test/sampler.jl +++ /dev/null @@ -1,211 +0,0 @@ -@testset "sampler.jl" begin - @testset "varnames with same symbol but different type" begin - struct S <: AbstractMCMC.AbstractSampler end - DynamicPPL.initialstep(rng, model, ::DynamicPPL.Sampler{S}, vi; kwargs...) = vi - @model function g() - y = (; a=1, b=2) - y.a ~ Normal() - return y.b ~ Normal() - end - model = g() - spl = DynamicPPL.Sampler(S()) - @test AbstractMCMC.step(Xoshiro(468), g(), spl) isa Any - end - - @testset "initial_state" begin - # Model is unused, but has to be a DynamicPPL.Model otherwise we won't hit our - # overloaded method. - @model f() = x ~ Normal() - model = f() - # This sampler just returns the state it was given as its 'sample' - struct S <: AbstractMCMC.AbstractSampler end - function AbstractMCMC.step( - rng::Random.AbstractRNG, - model::Model, - sampler::Sampler{<:S}, - state=nothing; - kwargs..., - ) - if state === nothing - s = rand() - return s, s - else - return state, state - end - end - spl = Sampler(S()) - - function AbstractMCMC.bundle_samples( - samples::Vector{Float64}, - model::Model, - sampler::Sampler{<:S}, - state, - chain_type::Type{MCMCChains.Chains}; - kwargs..., - ) - return MCMCChains.Chains(samples, [:x]; info=(samplerstate=state,)) - end - - N_iters, N_chains = 10, 3 - - @testset "single-chain sampling" begin - chn = sample(model, spl, N_iters; progress=false, chain_type=MCMCChains.Chains) - initial_value = chn[:x][1] - @test all(chn[:x] .== initial_value) # sanity check - chn2 = sample( - model, - spl, - N_iters; - progress=false, - initial_state=DynamicPPL.loadstate(chn), - chain_type=MCMCChains.Chains, - ) - @test all(chn2[:x] .== initial_value) - end - - @testset "multiple-chain sampling" begin - chn = sample( - model, - spl, - MCMCThreads(), - N_iters, - N_chains; - progress=false, - chain_type=MCMCChains.Chains, - ) - initial_value = chn[:x][1, :] - @test all(i -> chn[:x][i, :] == initial_value, 1:N_iters) # sanity check - chn2 = sample( - model, - spl, - MCMCThreads(), - N_iters, - N_chains; - progress=false, - initial_state=DynamicPPL.loadstate(chn), - chain_type=MCMCChains.Chains, - ) - @test all(i -> chn2[:x][i, :] == initial_value, 1:N_iters) - end - end - - @testset "Initial parameters" begin - # dummy algorithm that just returns initial value and does not perform any sampling - abstract type OnlyInitAlg end - struct OnlyInitAlgDefault <: OnlyInitAlg end - struct OnlyInitAlgUniform <: OnlyInitAlg end - function AbstractMCMC.step( - rng::Random.AbstractRNG, model::Model, ::OnlyInitAlg, kwargs... - ) - return vi, nothing - end - - # initial samplers - DynamicPPL.init_strategy(::OnlyInitAlgUniform) = UniformInit() - @test DynamicPPL.init_strategy(OnlyInitAlgDefault()) == PriorInit() - - for sampler in (OnlyInitAlgDefault(), OnlyInitAlgUniform()) - # model with one variable: initialization p = 0.2 - @model function coinflip() - p ~ Beta(1, 1) - return 10 ~ Binomial(25, p) - end - model = coinflip() - lptrue = logpdf(Binomial(25, 0.2), 10) - let inits = InitFromParams((; p=0.2)) - chain = sample(model, sampler, 1; initial_params=inits, progress=false) - @test chain[1].metadata.p.vals == [0.2] - @test getlogjoint(chain[1]) == lptrue - - # parallel sampling - chains = sample( - model, - sampler, - MCMCThreads(), - 1, - 10; - initial_params=fill(inits, 10), - progress=false, - ) - for c in chains - @test c[1].metadata.p.vals == [0.2] - @test getlogjoint(c[1]) == lptrue - end - end - - # check that Vector no longer works - @test_throws ArgumentError sample( - model, sampler, 1; initial_params=[4, -1], progress=false - ) - @test_throws ArgumentError sample( - model, sampler, 1; initial_params=[missing, -1], progress=false - ) - - # model with two variables: initialization s = 4, m = -1 - @model function twovars() - s ~ InverseGamma(2, 3) - return m ~ Normal(0, sqrt(s)) - end - model = twovars() - lptrue = logpdf(InverseGamma(2, 3), 4) + logpdf(Normal(0, 2), -1) - for inits in ( - InitFromParams((s=4, m=-1)), - (s=4, m=-1), - InitFromParams(Dict(@varname(s) => 4, @varname(m) => -1)), - Dict(@varname(s) => 4, @varname(m) => -1), - ) - chain = sample(model, sampler, 1; initial_params=inits, progress=false) - @test chain[1].metadata.s.vals == [4] - @test chain[1].metadata.m.vals == [-1] - @test getlogjoint(chain[1]) == lptrue - - # parallel sampling - chains = sample( - model, - sampler, - MCMCThreads(), - 1, - 10; - initial_params=fill(inits, 10), - progress=false, - ) - for c in chains - @test c[1].metadata.s.vals == [4] - @test c[1].metadata.m.vals == [-1] - @test getlogjoint(c[1]) == lptrue - end - end - - # set only m = -1 - for inits in ( - InitFromParams((; s=missing, m=-1)), - InitFromParams(Dict(@varname(s) => missing, @varname(m) => -1)), - (; s=missing, m=-1), - Dict(@varname(s) => missing, @varname(m) => -1), - InitFromParams((; m=-1)), - InitFromParams(Dict(@varname(m) => -1)), - (; m=-1), - Dict(@varname(m) => -1), - ) - chain = sample(model, sampler, 1; initial_params=inits, progress=false) - @test !ismissing(chain[1].metadata.s.vals[1]) - @test chain[1].metadata.m.vals == [-1] - - # parallel sampling - chains = sample( - model, - sampler, - MCMCThreads(), - 1, - 10; - initial_params=fill(inits, 10), - progress=false, - ) - for c in chains - @test !ismissing(c[1].metadata.s.vals[1]) - @test c[1].metadata.m.vals == [-1] - end - end - end - end -end From be0d2ade0d93de7b074b62e43fda2e54fe681060 Mon Sep 17 00:00:00 2001 From: Penelope Yong Date: Mon, 13 Oct 2025 20:38:13 +0100 Subject: [PATCH 3/5] forgot one function --- HISTORY.md | 1 + 1 file changed, 1 insertion(+) diff --git a/HISTORY.md b/HISTORY.md index 2f9dabb2c..af513939c 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -50,6 +50,7 @@ If you were using these, the corresponding replacements are: - `DynamicPPL.Sampler(S)`: just don't wrap `S`; but make sure `S` subtypes `AbstractMCMC.AbstractSampler` - `DynamicPPL.initialstep`: directly implement `AbstractMCMC.step` and `AbstractMCMC.step_warmup` as per the AbstractMCMC interface - `DynamicPPL.loadstate`: `Turing.loadstate` (will be introduced in the next version) + - `DynamicPPL.default_chain_type`: `Turing.default_chain_type` (will be introduced in the next version) - `DynamicPPL.initialsampler`: `Turing.init_strategy` (will be introduced in the next version; note that this function must return an `AbstractInitStrategy`, see above for explanation) - `DynamicPPL.default_varinfo`: `Turing.default_varinfo` (will be introduced in the next version) From 6efad430d2de88ca9e01ec63b934769bdf8956fa Mon Sep 17 00:00:00 2001 From: Penelope Yong Date: Thu, 16 Oct 2025 22:31:52 +0100 Subject: [PATCH 4/5] Move sampling test utils to Turing as well --- docs/src/api.md | 9 +--- src/test_utils.jl | 1 - src/test_utils/sampler.jl | 87 --------------------------------------- 3 files changed, 1 insertion(+), 96 deletions(-) delete mode 100644 src/test_utils/sampler.jl diff --git a/docs/src/api.md b/docs/src/api.md index bb67468d9..80970c0bb 100644 --- a/docs/src/api.md +++ b/docs/src/api.md @@ -243,14 +243,7 @@ DynamicPPL.TestUtils.AD.ADIncorrectException ## Demo models -DynamicPPL provides several demo models and helpers for testing samplers in the `DynamicPPL.TestUtils` submodule. - -```@docs -DynamicPPL.TestUtils.test_sampler -DynamicPPL.TestUtils.test_sampler_on_demo_models -DynamicPPL.TestUtils.test_sampler_continuous -DynamicPPL.TestUtils.marginal_mean_of_samples -``` +DynamicPPL provides several demo models in the `DynamicPPL.TestUtils` submodule. ```@docs DynamicPPL.TestUtils.DEMO_MODELS diff --git a/src/test_utils.jl b/src/test_utils.jl index 195345d60..f584055b3 100644 --- a/src/test_utils.jl +++ b/src/test_utils.jl @@ -17,7 +17,6 @@ include("test_utils/model_interface.jl") include("test_utils/models.jl") include("test_utils/contexts.jl") include("test_utils/varinfo.jl") -include("test_utils/sampler.jl") include("test_utils/ad.jl") end diff --git a/src/test_utils/sampler.jl b/src/test_utils/sampler.jl deleted file mode 100644 index 2101388fb..000000000 --- a/src/test_utils/sampler.jl +++ /dev/null @@ -1,87 +0,0 @@ -# sampler.jl -# ---------- -# -# Utilities to test samplers on models. - -using AbstractPPL: AbstractPPL - -""" - marginal_mean_of_samples(chain, varname) - -Return the mean of variable represented by `varname` in `chain`. -""" -marginal_mean_of_samples(chain, varname) = mean(Array(chain[Symbol(varname)])) - -""" - test_sampler(models, sampler, args...; kwargs...) - -Test that `sampler` produces correct marginal posterior means on each model in `models`. - -In short, this method iterates through `models`, calls `AbstractMCMC.sample` on the -`model` and `sampler` to produce a `chain`, and then checks `marginal_mean_of_samples(chain, vn)` -for every (leaf) varname `vn` against the corresponding value returned by -[`posterior_mean`](@ref) for each model. - -To change how comparison is done for a particular `chain` type, one can overload -[`marginal_mean_of_samples`](@ref) for the corresponding type. - -# Arguments -- `models`: A collection of instaces of [`DynamicPPL.Model`](@ref) to test on. -- `sampler`: The `AbstractMCMC.AbstractSampler` to test. -- `args...`: Arguments forwarded to `sample`. - -# Keyword arguments -- `varnames_filter`: A filter to apply to `varnames(model)`, allowing comparison for only - a subset of the varnames. -- `atol=1e-1`: Absolute tolerance used in `@test`. -- `rtol=1e-3`: Relative tolerance used in `@test`. -- `kwargs...`: Keyword arguments forwarded to `sample`. -""" -function test_sampler( - models, - sampler::AbstractMCMC.AbstractSampler, - args...; - varnames_filter=Returns(true), - atol=1e-1, - rtol=1e-3, - sampler_name=typeof(sampler), - kwargs..., -) - @testset "$(sampler_name) on $(nameof(model))" for model in models - chain = AbstractMCMC.sample(model, sampler, args...; kwargs...) - target_values = posterior_mean(model) - for vn in filter(varnames_filter, varnames(model)) - # We want to compare elementwise which can be achieved by - # extracting the leaves of the `VarName` and the corresponding value. - for vn_leaf in AbstractPPL.varname_leaves(vn, get(target_values, vn)) - target_value = get(target_values, vn_leaf) - chain_mean_value = marginal_mean_of_samples(chain, vn_leaf) - @test chain_mean_value ≈ target_value atol = atol rtol = rtol - end - end - end -end - -""" - test_sampler_on_demo_models(meanfunction, sampler, args...; kwargs...) - -Test `sampler` on every model in [`DEMO_MODELS`](@ref). - -This is just a proxy for `test_sampler(meanfunction, DEMO_MODELS, sampler, args...; kwargs...)`. -""" -function test_sampler_on_demo_models( - sampler::AbstractMCMC.AbstractSampler, args...; kwargs... -) - return test_sampler(DEMO_MODELS, sampler, args...; kwargs...) -end - -""" - test_sampler_continuous(sampler, args...; kwargs...) - -Test that `sampler` produces the correct marginal posterior means on all models in `demo_models`. - -As of right now, this is just an alias for [`test_sampler_on_demo_models`](@ref). -""" -function test_sampler_continuous(sampler::AbstractMCMC.AbstractSampler, args...; kwargs...) - return test_sampler_on_demo_models(sampler, args...; kwargs...) -end From 87eed7c63bf743ee853cfb4cfcca6797dc97df17 Mon Sep 17 00:00:00 2001 From: Penelope Yong Date: Fri, 17 Oct 2025 03:14:11 +0100 Subject: [PATCH 5/5] Update changelog to correctly reflect changes --- HISTORY.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index af513939c..eaacc7af2 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -50,9 +50,10 @@ If you were using these, the corresponding replacements are: - `DynamicPPL.Sampler(S)`: just don't wrap `S`; but make sure `S` subtypes `AbstractMCMC.AbstractSampler` - `DynamicPPL.initialstep`: directly implement `AbstractMCMC.step` and `AbstractMCMC.step_warmup` as per the AbstractMCMC interface - `DynamicPPL.loadstate`: `Turing.loadstate` (will be introduced in the next version) - - `DynamicPPL.default_chain_type`: `Turing.default_chain_type` (will be introduced in the next version) - - `DynamicPPL.initialsampler`: `Turing.init_strategy` (will be introduced in the next version; note that this function must return an `AbstractInitStrategy`, see above for explanation) - - `DynamicPPL.default_varinfo`: `Turing.default_varinfo` (will be introduced in the next version) + - `DynamicPPL.default_chain_type`: removed, just use the `chain_type` keyword argument directly + - `DynamicPPL.initialsampler`: `Turing.Inference.init_strategy` (will be introduced in the next version; note that this function must return an `AbstractInitStrategy`, see above for explanation) + - `DynamicPPL.default_varinfo`: `Turing.Inference.default_varinfo` (will be introduced in the next version) + - `DynamicPPL.TestUtils.test_sampler` and related methods: removed, please implement your own testing utilities as needed ### Simplification of the tilde-pipeline @@ -70,8 +71,8 @@ The only flag other than `"del"` that `Metadata` ever used was `"trans"`. Thus t ### Removal of `resume_from` -The `resume_from=chn` keyword argument to `sample` has been removed; please use `initial_state=DynamicPPL.loadstate(chn)` instead. -`loadstate` is exported from DynamicPPL. +The `resume_from=chn` keyword argument to `sample` has been removed; please use the `initial_state` argument instead. +`loadstate` will be exported from Turing in the next release of Turing. ### Change of default keytype of `pointwise_logdensities`