diff --git a/HISTORY.md b/HISTORY.md index c0e265fbf..691fae81f 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -72,6 +72,35 @@ The other case where one might use `PriorContext` was to use `@addlogprob!` to a Previously, this was accomplished by manually checking `__context__ isa DynamicPPL.PriorContext`. Now, you can write `@addlogprob (; logprior=x, loglikelihood=y)` to add `x` to the log-prior and `y` to the log-likelihood. +### Removal of `order` and `num_produce` + +The `VarInfo` type used to carry with it: + + - `num_produce`, an integer which recorded the number of observe tilde-statements that had been evaluated so far; and + - `order`, an integer per `VarName` which recorded the value of `num_produce` at the time that the variable was seen. + +These fields were used in particle samplers in Turing.jl. +In DynamicPPL 0.37, these fields and the associated functions have been removed: + + - `get_num_produce` + - `set_num_produce!!` + - `reset_num_produce!!` + - `increment_num_produce!!` + - `set_retained_vns_del!` + - `setorder!!` + +Because this is one of the more arcane features of DynamicPPL, some extra explanation is warranted. + +`num_produce` and `order`, along with the `del` flag in `VarInfo`, were used to control whether new values for variables were sampled during model execution. +For example, the particle Gibbs method has a _reference particle_, for which variables are never resampled. +However, if the reference particle is _forked_ (i.e., if the reference particle is selected by a resampling step multiple times and thereby copied), then the variables that have not yet been evaluated must be sampled anew to ensure that the new particle is independent of the reference particle. + +Previousy, this was accomplished by setting the `del` flag in the `VarInfo` object for all variables with `order` greater or equal to than `num_produce`. +Note that setting the `del` flag does not itself trigger a new value to be sampled; rather, it indicates that a new value should be sampled _if the variable is encountered again_. +[This Turing.jl PR](https://github.com/TuringLang/Turing.jl/pull/2629) changes the implementation to set the `del` flag for _all_ variables in the `VarInfo`. +Since the `del` flag only makes a difference when encountering a variable, this approach is entirely equivalent as long as the same variable is not seen multiple times in the model. +The interested reader is referred to that PR for more details. + **Internals** ### Accumulators @@ -83,7 +112,6 @@ This release overhauls how VarInfo objects track variables such as the log joint - `tilde_observe` and `observe` have been removed. `tilde_observe!!` still exists, and any contexts should modify its behaviour. We may further rework the call stack under `tilde_observe!!` in the near future. - `tilde_assume` no longer returns the log density of the current assumption as its second return value. We may further rework the `tilde_assume!!` call stack as well. - For literal observation statements like `0.0 ~ Normal(blahblah)` we used to call `tilde_observe!!` without the `vn` argument. This method no longer exists. Rather we call `tilde_observe!!` with `vn` set to `nothing`. - - `set/reset/increment_num_produce!` have become `set/reset/increment_num_produce!!` (note the second exclamation mark). They are no longer guaranteed to modify the `VarInfo` in place, and one should always use the return value. - `@addlogprob!` now _always_ adds to the log likelihood. Previously it added to the log probability that the execution context specified, e.g. the log prior when using `PriorContext`. - `getlogp` now returns a `NamedTuple` with keys `logprior` and `loglikelihood`. If you want the log joint probability, which is what `getlogp` used to return, use `getlogjoint`. - Correspondingly `setlogp!!` and `acclogp!!` should now be called with a `NamedTuple` with keys `logprior` and `loglikelihood`. The `acclogp!!` method with a single scalar value has been deprecated and falls back on `accloglikelihood!!`, and the single scalar version of `setlogp!!` has been removed. Corresponding setter/accumulator functions exist for the log prior as well. diff --git a/docs/src/api.md b/docs/src/api.md index 9237943c7..14b2447b5 100644 --- a/docs/src/api.md +++ b/docs/src/api.md @@ -334,17 +334,6 @@ unset_flag! is_flagged ``` -The following functions were used for sequential Monte Carlo methods. - -```@docs -get_num_produce -set_num_produce!! -increment_num_produce!! -reset_num_produce!! -setorder!! -set_retained_vns_del! -``` - ```@docs Base.empty! ``` @@ -369,7 +358,6 @@ DynamicPPL provides the following default accumulators. LogPriorAccumulator LogJacobianAccumulator LogLikelihoodAccumulator -VariableOrderAccumulator ``` ### Common API diff --git a/src/DynamicPPL.jl b/src/DynamicPPL.jl index 4a13c9878..f190c7605 100644 --- a/src/DynamicPPL.jl +++ b/src/DynamicPPL.jl @@ -51,7 +51,6 @@ export AbstractVarInfo, LogLikelihoodAccumulator, LogPriorAccumulator, LogJacobianAccumulator, - VariableOrderAccumulator, push!!, empty!!, subset, @@ -72,15 +71,9 @@ export AbstractVarInfo, acclogprior!!, accloglikelihood!!, resetlogp!!, - get_num_produce, - set_num_produce!!, - reset_num_produce!!, - increment_num_produce!!, - set_retained_vns_del!, is_flagged, set_flag!, unset_flag!, - setorder!!, istrans, link, link!!, diff --git a/src/abstract_varinfo.jl b/src/abstract_varinfo.jl index caf6dc16c..7940f20e6 100644 --- a/src/abstract_varinfo.jl +++ b/src/abstract_varinfo.jl @@ -441,24 +441,6 @@ function resetlogp!!(vi::AbstractVarInfo) return vi end -""" - setorder!!(vi::AbstractVarInfo, vn::VarName, index::Integer) - -Set the `order` of `vn` in `vi` to `index`, where `order` is the number of `observe -statements run before sampling `vn`. -""" -function setorder!!(vi::AbstractVarInfo, vn::VarName, index::Integer) - return map_accumulator!!(acc -> (acc.order[vn] = index; acc), vi, Val(:VariableOrder)) -end - -""" - getorder(vi::VarInfo, vn::VarName) - -Get the `order` of `vn` in `vi`, where `order` is the number of `observe` statements -run before sampling `vn`. -""" -getorder(vi::AbstractVarInfo, vn::VarName) = getacc(vi, Val(:VariableOrder)).order[vn] - # Variables and their realizations. @doc """ keys(vi::AbstractVarInfo) @@ -509,8 +491,7 @@ function getindex_internal end @doc """ empty!!(vi::AbstractVarInfo) -Empty the fields of `vi.metadata` and reset `vi.logp[]` and `vi.num_produce[]` to -zeros. +Empty `vi` of variables and reset any `logp` accumulators zeros. This is useful when using a sampling algorithm that assumes an empty `vi`, e.g. `SMC`. """ BangBang.empty!! @@ -1068,43 +1049,6 @@ function invlink_with_logpdf(vi::AbstractVarInfo, vn::VarName, dist, y) return x, logpdf(dist, x) + logjac end -""" - get_num_produce(vi::AbstractVarInfo) - -Return the `num_produce` of `vi`. -""" -get_num_produce(vi::AbstractVarInfo) = getacc(vi, Val(:VariableOrder)).num_produce - -""" - set_num_produce!!(vi::AbstractVarInfo, n::Int) - -Set the `num_produce` field of `vi` to `n`. -""" -function set_num_produce!!(vi::AbstractVarInfo, n::Integer) - if hasacc(vi, Val(:VariableOrder)) - acc = getacc(vi, Val(:VariableOrder)) - acc = VariableOrderAccumulator(n, acc.order) - else - acc = VariableOrderAccumulator(n) - end - return setacc!!(vi, acc) -end - -""" - increment_num_produce!!(vi::AbstractVarInfo) - -Add 1 to `num_produce` in `vi`. -""" -increment_num_produce!!(vi::AbstractVarInfo) = - map_accumulator!!(increment, vi, Val(:VariableOrder)) - -""" - reset_num_produce!!(vi::AbstractVarInfo) - -Reset the value of `num_produce` in `vi` to 0. -""" -reset_num_produce!!(vi::AbstractVarInfo) = set_num_produce!!(vi, zero(get_num_produce(vi))) - """ from_internal_transform(varinfo::AbstractVarInfo, vn::VarName[, dist]) diff --git a/src/accumulators.jl b/src/accumulators.jl index b560307b7..0dcf9c7cf 100644 --- a/src/accumulators.jl +++ b/src/accumulators.jl @@ -30,13 +30,6 @@ To be able to work with multi-threading, it should also implement: - `split(acc::T)` - `combine(acc::T, acc2::T)` -If two accumulators of the same type should be merged in some non-trivial way, other than -always keeping the second one over the first, `merge(acc1::T, acc2::T)` should be defined. - -If limiting the accumulator to a subset of `VarName`s is a meaningful operation and should -do something other than copy the original accumulator, then -`subset(acc::T, vns::AbstractVector{<:VarnName})` should be defined.` - See the documentation for each of these functions for more details. """ abstract type AbstractAccumulator end @@ -120,24 +113,6 @@ used by various AD backends, should implement a method for this function. """ convert_eltype(::Type, acc::AbstractAccumulator) = acc -""" - subset(acc::AbstractAccumulator, vns::AbstractVector{<:VarName}) - -Return a new accumulator that only contains the information for the `VarName`s in `vns`. - -By default returns a copy of `acc`. Subtypes should override this behaviour as needed. -""" -subset(acc::AbstractAccumulator, ::AbstractVector{<:VarName}) = copy(acc) - -""" - merge(acc1::AbstractAccumulator, acc2::AbstractAccumulator) - -Merge two accumulators of the same type. Returns a new accumulator of the same type. - -By default returns a copy of `acc2`. Subtypes should override this behaviour as needed. -""" -Base.merge(acc1::AbstractAccumulator, acc2::AbstractAccumulator) = copy(acc2) - """ AccumulatorTuple{N,T<:NamedTuple} @@ -183,50 +158,6 @@ function Base.convert(::Type{AccumulatorTuple{N,T}}, accs::AccumulatorTuple{N}) return AccumulatorTuple(convert(T, accs.nt)) end -""" - subset(at::AccumulatorTuple, vns::AbstractVector{<:VarName}) - -Replace each accumulator `acc` in `at` with `subset(acc, vns)`. -""" -function subset(at::AccumulatorTuple, vns::AbstractVector{<:VarName}) - return AccumulatorTuple(map(Base.Fix2(subset, vns), at.nt)) -end - -""" - _joint_keys(nt1::NamedTuple, nt2::NamedTuple) - -A helper function that returns three tuples of keys given two `NamedTuple`s: -The keys only in `nt1`, only in `nt2`, and in both, and in that order. - -Implemented as a generated function to enable constant propagation of the result in `merge`. -""" -@generated function _joint_keys( - nt1::NamedTuple{names1}, nt2::NamedTuple{names2} -) where {names1,names2} - only_in_nt1 = tuple(setdiff(names1, names2)...) - only_in_nt2 = tuple(setdiff(names2, names1)...) - in_both = tuple(intersect(names1, names2)...) - return :($only_in_nt1, $only_in_nt2, $in_both) -end - -""" - merge(at1::AccumulatorTuple, at2::AccumulatorTuple) - -Merge two `AccumulatorTuple`s. - -For any `accumulator_name` that exists in both `at1` and `at2`, we call `merge` on the two -accumulators themselves. Other accumulators are copied. -""" -function Base.merge(at1::AccumulatorTuple, at2::AccumulatorTuple) - keys_in_at1, keys_in_at2, keys_in_both = _joint_keys(at1.nt, at2.nt) - accs_in_at1 = (getfield(at1.nt, key) for key in keys_in_at1) - accs_in_at2 = (getfield(at2.nt, key) for key in keys_in_at2) - accs_in_both = ( - merge(getfield(at1.nt, key), getfield(at2.nt, key)) for key in keys_in_both - ) - return AccumulatorTuple(accs_in_at1..., accs_in_both..., accs_in_at2...) -end - """ setacc!!(at::AccumulatorTuple, acc::AbstractAccumulator) diff --git a/src/default_accumulators.jl b/src/default_accumulators.jl index 8d51a8431..2b505f1bb 100644 --- a/src/default_accumulators.jl +++ b/src/default_accumulators.jl @@ -166,96 +166,6 @@ function accumulate_observe!!(acc::LogLikelihoodAccumulator, right, left, vn) return acclogp(acc, Distributions.loglikelihood(right, left)) end -""" - VariableOrderAccumulator{T} <: AbstractAccumulator - -An accumulator that tracks the order of variables in a `VarInfo`. - -This doesn't track the full ordering, but rather how many observations have taken place -before the assume statement for each variable. This is needed for particle methods, where -the model is segmented into parts by each observation, and we need to know which part each -assume statement is in. - -# Fields -$(TYPEDFIELDS) -""" -struct VariableOrderAccumulator{Eltype<:Integer,VNType<:VarName} <: AbstractAccumulator - "the number of observations" - num_produce::Eltype - "mapping of variable names to their order in the model" - order::Dict{VNType,Eltype} -end - -""" - VariableOrderAccumulator{T<:Integer}(n=zero(T)) - -Create a new `VariableOrderAccumulator` with the number of observations set to `n`. -""" -VariableOrderAccumulator{T}(n=zero(T)) where {T<:Integer} = - VariableOrderAccumulator(convert(T, n), Dict{VarName,T}()) -VariableOrderAccumulator(n) = VariableOrderAccumulator{typeof(n)}(n) -VariableOrderAccumulator() = VariableOrderAccumulator{Int}() - -function Base.copy(acc::VariableOrderAccumulator) - return VariableOrderAccumulator(acc.num_produce, copy(acc.order)) -end - -function Base.show(io::IO, acc::VariableOrderAccumulator) - return print( - io, "VariableOrderAccumulator($(string(acc.num_produce)), $(repr(acc.order)))" - ) -end - -function Base.:(==)(acc1::VariableOrderAccumulator, acc2::VariableOrderAccumulator) - return acc1.num_produce == acc2.num_produce && acc1.order == acc2.order -end - -function Base.isequal(acc1::VariableOrderAccumulator, acc2::VariableOrderAccumulator) - return isequal(acc1.num_produce, acc2.num_produce) && isequal(acc1.order, acc2.order) -end - -function Base.hash(acc::VariableOrderAccumulator, h::UInt) - return hash((VariableOrderAccumulator, acc.num_produce, acc.order), h) -end - -accumulator_name(::Type{<:VariableOrderAccumulator}) = :VariableOrder - -split(acc::VariableOrderAccumulator) = copy(acc) - -function combine(acc::VariableOrderAccumulator, acc2::VariableOrderAccumulator) - # Note that assumptions are not allowed in parallelised blocks, and thus the - # dictionaries should be identical. - return VariableOrderAccumulator( - max(acc.num_produce, acc2.num_produce), merge(acc.order, acc2.order) - ) -end - -function increment(acc::VariableOrderAccumulator) - return VariableOrderAccumulator(acc.num_produce + oneunit(acc.num_produce), acc.order) -end - -function accumulate_assume!!(acc::VariableOrderAccumulator, val, logjac, vn, right) - acc.order[vn] = acc.num_produce - return acc -end -accumulate_observe!!(acc::VariableOrderAccumulator, right, left, vn) = increment(acc) - -function Base.convert( - ::Type{VariableOrderAccumulator{ElType,VnType}}, acc::VariableOrderAccumulator -) where {ElType,VnType} - order = Dict{VnType,ElType}() - for (k, v) in acc.order - order[convert(VnType, k)] = convert(ElType, v) - end - return VariableOrderAccumulator(convert(ElType, acc.num_produce), order) -end - -# TODO(mhauru) -# We ignore the convert_eltype calls for VariableOrderAccumulator, by letting them fallback on -# convert_eltype(::AbstractAccumulator, ::Type). This is because they are only used to -# deal with dual number types of AD backends, which shouldn't concern VariableOrderAccumulator. This is -# horribly hacky and should be fixed. See also comment in `unflatten` in `src/varinfo.jl`. - function default_accumulators( ::Type{FloatT}=LogProbType, ::Type{IntT}=Int ) where {FloatT,IntT} @@ -263,22 +173,5 @@ function default_accumulators( LogPriorAccumulator{FloatT}(), LogJacobianAccumulator{FloatT}(), LogLikelihoodAccumulator{FloatT}(), - VariableOrderAccumulator{IntT}(), ) end - -function subset(acc::VariableOrderAccumulator, vns::AbstractVector{<:VarName}) - order = filter(pair -> any(subsumes(vn, first(pair)) for vn in vns), acc.order) - return VariableOrderAccumulator(acc.num_produce, order) -end - -""" - merge(acc1::VariableOrderAccumulator, acc2::VariableOrderAccumulator) - -Merge two `VariableOrderAccumulator` instances. - -The `num_produce` field of the return value is the `num_produce` of `acc2`. -""" -function Base.merge(acc1::VariableOrderAccumulator, acc2::VariableOrderAccumulator) - return VariableOrderAccumulator(acc2.num_produce, merge(acc1.order, acc2.order)) -end diff --git a/src/simple_varinfo.jl b/src/simple_varinfo.jl index 4997b4b8d..ad22bf52d 100644 --- a/src/simple_varinfo.jl +++ b/src/simple_varinfo.jl @@ -122,7 +122,7 @@ Evaluation in transformed space of course also works: ```jldoctest simplevarinfo-general julia> vi = DynamicPPL.settrans!!(SimpleVarInfo((x = -1.0,)), true) -Transformed SimpleVarInfo((x = -1.0,), (LogPrior = LogPriorAccumulator(0.0), LogJacobian = LogJacobianAccumulator(0.0), LogLikelihood = LogLikelihoodAccumulator(0.0), VariableOrder = VariableOrderAccumulator(0, Dict{VarName, Int64}()))) +Transformed SimpleVarInfo((x = -1.0,), (LogPrior = LogPriorAccumulator(0.0), LogJacobian = LogJacobianAccumulator(0.0), LogLikelihood = LogLikelihoodAccumulator(0.0))) julia> # (✓) Positive probability mass on negative numbers! getlogjoint_internal(last(DynamicPPL.evaluate!!(m, vi))) @@ -130,7 +130,7 @@ julia> # (✓) Positive probability mass on negative numbers! julia> # While if we forget to indicate that it's transformed: vi = DynamicPPL.settrans!!(SimpleVarInfo((x = -1.0,)), false) -SimpleVarInfo((x = -1.0,), (LogPrior = LogPriorAccumulator(0.0), LogJacobian = LogJacobianAccumulator(0.0), LogLikelihood = LogLikelihoodAccumulator(0.0), VariableOrder = VariableOrderAccumulator(0, Dict{VarName, Int64}()))) +SimpleVarInfo((x = -1.0,), (LogPrior = LogPriorAccumulator(0.0), LogJacobian = LogJacobianAccumulator(0.0), LogLikelihood = LogLikelihoodAccumulator(0.0))) julia> # (✓) No probability mass on negative numbers! getlogjoint_internal(last(DynamicPPL.evaluate!!(m, vi))) @@ -418,7 +418,7 @@ Base.eltype(::SimpleOrThreadSafeSimple{<:Any,V}) where {V} = V # `subset` function subset(varinfo::SimpleVarInfo, vns::AbstractVector{<:VarName}) return SimpleVarInfo( - _subset(varinfo.values, vns), subset(getaccs(varinfo), vns), varinfo.transformation + _subset(varinfo.values, vns), map(copy, getaccs(varinfo)), varinfo.transformation ) end @@ -456,7 +456,7 @@ _subset(x::VarNamedVector, vns) = subset(x, vns) # `merge` function Base.merge(varinfo_left::SimpleVarInfo, varinfo_right::SimpleVarInfo) values = merge(varinfo_left.values, varinfo_right.values) - accs = merge(getaccs(varinfo_left), getaccs(varinfo_right)) + accs = map(copy, getaccs(varinfo_right)) transformation = merge_transformations( varinfo_left.transformation, varinfo_right.transformation ) diff --git a/src/threadsafe.jl b/src/threadsafe.jl index 5f0a6d3e5..450dd2c38 100644 --- a/src/threadsafe.jl +++ b/src/threadsafe.jl @@ -73,23 +73,8 @@ function BangBang.push!!(vi::ThreadSafeVarInfo, vn::VarName, r, dist::Distributi return Accessors.@set vi.varinfo = push!!(vi.varinfo, vn, r, dist) end -# TODO(mhauru) Why these short-circuits? Why not use the thread-specific ones? -get_num_produce(vi::ThreadSafeVarInfo) = get_num_produce(vi.varinfo) -function increment_num_produce!!(vi::ThreadSafeVarInfo) - return ThreadSafeVarInfo(increment_num_produce!!(vi.varinfo), vi.accs_by_thread) -end -function reset_num_produce!!(vi::ThreadSafeVarInfo) - return ThreadSafeVarInfo(reset_num_produce!!(vi.varinfo), vi.accs_by_thread) -end -function set_num_produce!!(vi::ThreadSafeVarInfo, n::Int) - return ThreadSafeVarInfo(set_num_produce!!(vi.varinfo, n), vi.accs_by_thread) -end - syms(vi::ThreadSafeVarInfo) = syms(vi.varinfo) -function setorder!!(vi::ThreadSafeVarInfo, vn::VarName, index::Int) - return ThreadSafeVarInfo(setorder!!(vi.varinfo, vn, index), vi.accs_by_thread) -end setval!(vi::ThreadSafeVarInfo, val, vn::VarName) = setval!(vi.varinfo, val, vn) keys(vi::ThreadSafeVarInfo) = keys(vi.varinfo) @@ -184,10 +169,6 @@ function vector_getranges(vi::ThreadSafeVarInfo, vns::Vector{<:VarName}) return vector_getranges(vi.varinfo, vns) end -function set_retained_vns_del!(vi::ThreadSafeVarInfo) - return set_retained_vns_del!(vi.varinfo) -end - isempty(vi::ThreadSafeVarInfo) = isempty(vi.varinfo) function BangBang.empty!!(vi::ThreadSafeVarInfo) return resetlogp!!(Accessors.@set(vi.varinfo = empty!!(vi.varinfo))) diff --git a/src/varinfo.jl b/src/varinfo.jl index b364f5bcc..e115a6799 100644 --- a/src/varinfo.jl +++ b/src/varinfo.jl @@ -447,7 +447,7 @@ end function subset(varinfo::VarInfo, vns::AbstractVector{<:VarName}) metadata = subset(varinfo.metadata, vns) - return VarInfo(metadata, subset(getaccs(varinfo), vns)) + return VarInfo(metadata, map(copy, getaccs(varinfo))) end function subset(metadata::NamedTuple, vns::AbstractVector{<:VarName}) @@ -528,7 +528,7 @@ end function _merge(varinfo_left::VarInfo, varinfo_right::VarInfo) metadata = merge_metadata(varinfo_left.metadata, varinfo_right.metadata) - accs = merge(getaccs(varinfo_left), getaccs(varinfo_right)) + accs = map(copy, getaccs(varinfo_right)) return VarInfo(metadata, accs) end @@ -884,7 +884,6 @@ end function BangBang.empty!!(vi::VarInfo) _empty!(vi.metadata) vi = resetlogp!!(vi) - vi = reset_num_produce!!(vi) return vi end @@ -1827,28 +1826,6 @@ function unset_flag!(vnv::VarNamedVector, ::VarName, flag::String, ignorable::Bo return vnv end -""" - set_retained_vns_del!(vi::VarInfo) - -Set the `"del"` flag of variables in `vi` with `order > num_produce` to `true`. If -`num_produce` is `0`, _all_ variables will have their `"del"` flag set to `true`. - -Will error if `vi` does not have an accumulator for `VariableOrder`. -""" -function set_retained_vns_del!(vi::VarInfo) - if !hasacc(vi, Val(:VariableOrder)) - msg = "`vi` must have an accumulator for VariableOrder to set the `del` flag." - throw(ArgumentError(msg)) - end - num_produce = get_num_produce(vi) - for vn in keys(vi) - if num_produce == 0 || getorder(vi, vn) > num_produce - set_flag!(vi, vn, "del") - end - end - return nothing -end - # TODO: Maybe rename or something? """ _apply!(kernel!, vi::VarInfo, values, keys) diff --git a/src/varnamedvector.jl b/src/varnamedvector.jl index 5de0874c9..d756a4922 100644 --- a/src/varnamedvector.jl +++ b/src/varnamedvector.jl @@ -766,13 +766,6 @@ function update_internal!( return nothing end -# TODO(mhauru) The num_produce argument is used by Particle Gibbs. -# Remove this method as soon as possible. -function BangBang.push!(vnv::VarNamedVector, vn, val, dist, num_produce) - f = from_vec_transform(dist) - return setindex_internal!(vnv, tovec(val), vn, f) -end - # BangBang versions of the above functions. # The only difference is that update_internal!! and insert_internal!! check whether the # container types of the VarNamedVector vector need to be expanded to accommodate the new @@ -963,13 +956,6 @@ function BangBang.push!!(vnv::VarNamedVector, pair::Pair) return setindex!!(vnv, val, vn) end -# TODO(mhauru) The num_produce argument is used by Particle Gibbs. -# Remove this method as soon as possible. -function BangBang.push!!(vnv::VarNamedVector, vn, val, dist, num_produce) - f = from_vec_transform(dist) - return setindex_internal!!(vnv, tovec(val), vn, f) -end - function Base.empty!(vnv::VarNamedVector) # TODO: Or should the semantics be different, e.g. keeping `varnames`? empty!(vnv.varname_to_index) diff --git a/test/accumulators.jl b/test/accumulators.jl index d84fbf43d..df20f2c11 100644 --- a/test/accumulators.jl +++ b/test/accumulators.jl @@ -7,13 +7,11 @@ using DynamicPPL: AccumulatorTuple, LogLikelihoodAccumulator, LogPriorAccumulator, - VariableOrderAccumulator, accumulate_assume!!, accumulate_observe!!, combine, convert_eltype, getacc, - increment, map_accumulator, setacc!!, split @@ -31,11 +29,6 @@ using DynamicPPL: LogLikelihoodAccumulator{Float64}() == LogLikelihoodAccumulator{Float64}(0.0) == zero(LogLikelihoodAccumulator(1.0)) - @test VariableOrderAccumulator(0) == - VariableOrderAccumulator() == - VariableOrderAccumulator{Int}() == - VariableOrderAccumulator{Int}(0) == - VariableOrderAccumulator(0, Dict{VarName,Int}()) end @testset "addition and incrementation" begin @@ -45,19 +38,14 @@ using DynamicPPL: LogLikelihoodAccumulator(2.0f0) @test acclogp(LogLikelihoodAccumulator(1.0), 1.0f0) == LogLikelihoodAccumulator(2.0) - @test increment(VariableOrderAccumulator()) == VariableOrderAccumulator(1) - @test increment(VariableOrderAccumulator{UInt8}()) == - VariableOrderAccumulator{UInt8}(1) end @testset "split and combine" begin for acc in [ LogPriorAccumulator(1.0), LogLikelihoodAccumulator(1.0), - VariableOrderAccumulator(1), LogPriorAccumulator(1.0f0), LogLikelihoodAccumulator(1.0f0), - VariableOrderAccumulator(UInt8(1)), ] @test combine(acc, split(acc)) == acc end @@ -69,9 +57,6 @@ using DynamicPPL: @test convert( LogLikelihoodAccumulator{Float32}, LogLikelihoodAccumulator(1.0) ) == LogLikelihoodAccumulator{Float32}(1.0f0) - @test convert( - VariableOrderAccumulator{UInt8,VarName}, VariableOrderAccumulator(1) - ) == VariableOrderAccumulator{UInt8}(1) @test convert_eltype(Float32, LogPriorAccumulator(1.0)) == LogPriorAccumulator{Float32}(1.0f0) @@ -91,8 +76,6 @@ using DynamicPPL: @test accumulate_assume!!( LogLikelihoodAccumulator(1.0), val, logjac, vn, dist ) == LogLikelihoodAccumulator(1.0) - @test accumulate_assume!!(VariableOrderAccumulator(1), val, logjac, vn, dist) == - VariableOrderAccumulator(1, Dict{VarName,Int}((vn => 1))) end @testset "accumulate_observe" begin @@ -105,75 +88,6 @@ using DynamicPPL: LogJacobianAccumulator(1.0) @test accumulate_observe!!(LogLikelihoodAccumulator(1.0), right, left, vn) == LogLikelihoodAccumulator(1.0 + logpdf(right, left)) - @test accumulate_observe!!(VariableOrderAccumulator(1), right, left, vn) == - VariableOrderAccumulator(2) - end - - @testset "merge" begin - @test merge(LogPriorAccumulator(1.0), LogPriorAccumulator(2.0)) == - LogPriorAccumulator(2.0) - @test merge(LogJacobianAccumulator(1.0), LogJacobianAccumulator(2.0)) == - LogJacobianAccumulator(2.0) - @test merge(LogLikelihoodAccumulator(1.0), LogLikelihoodAccumulator(2.0)) == - LogLikelihoodAccumulator(2.0) - - @test merge( - VariableOrderAccumulator(1, Dict{VarName,Int}()), - VariableOrderAccumulator(2, Dict{VarName,Int}()), - ) == VariableOrderAccumulator(2, Dict{VarName,Int}()) - @test merge( - VariableOrderAccumulator( - 2, Dict{VarName,Int}((@varname(a) => 1, @varname(b) => 2)) - ), - VariableOrderAccumulator( - 1, Dict{VarName,Int}((@varname(a) => 2, @varname(c) => 3)) - ), - ) == VariableOrderAccumulator( - 1, Dict{VarName,Int}((@varname(a) => 2, @varname(b) => 2, @varname(c) => 3)) - ) - end - - @testset "subset" begin - @test subset(LogPriorAccumulator(1.0), VarName[]) == LogPriorAccumulator(1.0) - @test subset(LogJacobianAccumulator(1.0), VarName[]) == - LogJacobianAccumulator(1.0) - @test subset(LogLikelihoodAccumulator(1.0), VarName[]) == - LogLikelihoodAccumulator(1.0) - - @test subset( - VariableOrderAccumulator(1, Dict{VarName,Int}()), - VarName[@varname(a), @varname(b)], - ) == VariableOrderAccumulator(1, Dict{VarName,Int}()) - @test subset( - VariableOrderAccumulator( - 2, Dict{VarName,Int}((@varname(a) => 1, @varname(b) => 2)) - ), - VarName[@varname(a)], - ) == VariableOrderAccumulator(2, Dict{VarName,Int}((@varname(a) => 1))) - @test subset( - VariableOrderAccumulator( - 2, Dict{VarName,Int}((@varname(a) => 1, @varname(b) => 2)) - ), - VarName[], - ) == VariableOrderAccumulator(2, Dict{VarName,Int}()) - @test subset( - VariableOrderAccumulator( - 2, - Dict{VarName,Int}(( - @varname(a) => 1, - @varname(a.b.c) => 2, - @varname(a.b.c.d[1]) => 2, - @varname(b) => 3, - @varname(c[1]) => 4, - )), - ), - VarName[@varname(a.b), @varname(b)], - ) == VariableOrderAccumulator( - 2, - Dict{VarName,Int}(( - @varname(a.b.c) => 2, @varname(a.b.c.d[1]) => 2, @varname(b) => 3 - )), - ) end end @@ -183,7 +97,6 @@ using DynamicPPL: lp_f32 = LogPriorAccumulator(1.0f0) ll_f64 = LogLikelihoodAccumulator(1.0) ll_f32 = LogLikelihoodAccumulator(1.0f0) - vo_i64 = VariableOrderAccumulator(1) @testset "constructors" begin @test AccumulatorTuple(lp_f64, ll_f64) == AccumulatorTuple((lp_f64, ll_f64)) @@ -197,22 +110,21 @@ using DynamicPPL: end @testset "basic operations" begin - at_all64 = AccumulatorTuple(lp_f64, ll_f64, vo_i64) + at_all64 = AccumulatorTuple(lp_f64, ll_f64) @test at_all64[:LogPrior] == lp_f64 @test at_all64[:LogLikelihood] == ll_f64 - @test at_all64[:VariableOrder] == vo_i64 - @test haskey(AccumulatorTuple(vo_i64), Val(:VariableOrder)) - @test ~haskey(AccumulatorTuple(vo_i64), Val(:LogPrior)) - @test length(AccumulatorTuple(lp_f64, ll_f64, vo_i64)) == 3 - @test keys(at_all64) == (:LogPrior, :LogLikelihood, :VariableOrder) - @test collect(at_all64) == [lp_f64, ll_f64, vo_i64] + @test haskey(AccumulatorTuple(lp_f64), Val(:LogPrior)) + @test ~haskey(AccumulatorTuple(lp_f64), Val(:LogLikelihood)) + @test length(AccumulatorTuple(lp_f64, ll_f64)) == 2 + @test keys(at_all64) == (:LogPrior, :LogLikelihood) + @test collect(at_all64) == [lp_f64, ll_f64] # Replace the existing LogPriorAccumulator @test setacc!!(at_all64, lp_f32)[:LogPrior] == lp_f32 # Check that setacc!! didn't modify the original - @test at_all64 == AccumulatorTuple(lp_f64, ll_f64, vo_i64) + @test at_all64 == AccumulatorTuple(lp_f64, ll_f64) # Add a new accumulator type. @test setacc!!(AccumulatorTuple(lp_f64), ll_f64) == AccumulatorTuple(lp_f64, ll_f64) @@ -240,52 +152,6 @@ using DynamicPPL: acc -> convert_eltype(Float64, acc), accs, Val(:LogLikelihood) ) == AccumulatorTuple(lp_f32, LogLikelihoodAccumulator(1.0)) end - - @testset "merge" begin - vo1 = VariableOrderAccumulator( - 1, Dict{VarName,Int}(@varname(a) => 1, @varname(b) => 1) - ) - vo2 = VariableOrderAccumulator( - 2, Dict{VarName,Int}(@varname(a) => 2, @varname(c) => 2) - ) - accs1 = AccumulatorTuple(lp_f64, ll_f64, vo1) - accs2 = AccumulatorTuple(lp_f32, vo2) - @test merge(accs1, accs2) == AccumulatorTuple( - ll_f64, - lp_f32, - VariableOrderAccumulator( - 2, - Dict{VarName,Int}(@varname(a) => 2, @varname(b) => 1, @varname(c) => 2), - ), - ) - @test merge(AccumulatorTuple(), accs1) == accs1 - @test merge(accs1, AccumulatorTuple()) == accs1 - @test merge(accs1, accs1) == accs1 - end - - @testset "subset" begin - accs = AccumulatorTuple( - lp_f64, - ll_f64, - VariableOrderAccumulator( - 1, - Dict{VarName,Int}( - @varname(a.b) => 1, @varname(a.b[1]) => 2, @varname(b) => 1 - ), - ), - ) - - @test subset(accs, VarName[]) == AccumulatorTuple( - lp_f64, ll_f64, VariableOrderAccumulator(1, Dict{VarName,Int}()) - ) - @test subset(accs, VarName[@varname(a)]) == AccumulatorTuple( - lp_f64, - ll_f64, - VariableOrderAccumulator( - 1, Dict{VarName,Int}(@varname(a.b) => 1, @varname(a.b[1]) => 2) - ), - ) - end end end diff --git a/test/varinfo.jl b/test/varinfo.jl index bd0c0a987..202ddc1b2 100644 --- a/test/varinfo.jl +++ b/test/varinfo.jl @@ -21,13 +21,11 @@ function randr(vi::DynamicPPL.VarInfo, vn::VarName, dist::Distribution) if !haskey(vi, vn) r = rand(dist) push!!(vi, vn, r, dist) - vi = DynamicPPL.setorder!!(vi, vn, DynamicPPL.get_num_produce(vi)) r elseif DynamicPPL.is_flagged(vi, vn, "del") DynamicPPL.unset_flag!(vi, vn, "del") r = rand(dist) vi[vn] = DynamicPPL.tovec(r) - vi = DynamicPPL.setorder!!(vi, vn, DynamicPPL.get_num_produce(vi)) r else vi[vn] @@ -161,9 +159,7 @@ end lp_d = logpdf(Normal(), values.d) m = demo() | (; c=values.c, d=values.d) - vi = DynamicPPL.reset_num_produce!!( - DynamicPPL.unflatten(VarInfo(m), collect(values)) - ) + vi = DynamicPPL.unflatten(VarInfo(m), collect(values)) vi = last(DynamicPPL.evaluate!!(m, deepcopy(vi))) @test getlogprior(vi) == lp_a + lp_b @@ -171,7 +167,6 @@ end @test getloglikelihood(vi) == lp_c + lp_d @test getlogp(vi) == (; logprior=lp_a + lp_b, logjac=0.0, loglikelihood=lp_c + lp_d) @test getlogjoint(vi) == lp_a + lp_b + lp_c + lp_d - @test get_num_produce(vi) == 2 @test begin vi = acclogprior!!(vi, 1.0) getlogprior(vi) == lp_a + lp_b + 1.0 @@ -213,7 +208,6 @@ end @test_throws r"has no field `?LogLikelihood" getloglikelihood(vi) @test_throws r"has no field `?LogJacobian" getlogp(vi) @test_throws r"has no field `?LogLikelihood" getlogjoint(vi) - @test_throws r"has no field `?VariableOrder" get_num_produce(vi) @test begin vi = acclogprior!!(vi, 1.0) getlogprior(vi) == lp_a + lp_b + 1.0 @@ -223,19 +217,6 @@ end getlogprior(vi) == -1.0 end - vi = last( - DynamicPPL.evaluate!!( - m, DynamicPPL.setaccs!!(deepcopy(vi), (VariableOrderAccumulator(),)) - ), - ) - # need regex because 1.11 and 1.12 throw different errors (in 1.12 the - # missing field is surrounded by backticks) - @test_throws r"has no field `?LogPrior" getlogprior(vi) - @test_throws r"has no field `?LogLikelihood" getloglikelihood(vi) - @test_throws r"has no field `?LogPrior" getlogp(vi) - @test_throws r"has no field `?LogPrior" getlogjoint(vi) - @test get_num_produce(vi) == 2 - # Test evaluating without any accumulators. vi = last(DynamicPPL.evaluate!!(m, DynamicPPL.setaccs!!(deepcopy(vi), ()))) # need regex because 1.11 and 1.12 throw different errors (in 1.12 the @@ -244,8 +225,6 @@ end @test_throws r"has no field `?LogLikelihood" getloglikelihood(vi) @test_throws r"has no field `?LogPrior" getlogp(vi) @test_throws r"has no field `?LogPrior" getlogjoint(vi) - @test_throws r"has no field `?VariableOrder" get_num_produce(vi) - @test_throws r"has no field `?VariableOrder" reset_num_produce!!(vi) end @testset "flags" begin @@ -494,7 +473,7 @@ end # Check that instantiating the model using SampleFromUniform does not # perform linking # Note (penelopeysm): The purpose of using SampleFromUniform (SFU) - # specifically in this test is because SFU samples from the linked + # specifically in this test is because SFU samples from the linked # distribution i.e. in unconstrained space. However, it does this not # by linking the varinfo but by transforming the distributions on the # fly. That's why it's worth specifically checking that it can do this @@ -1051,127 +1030,6 @@ end end end - @testset "orders" begin - @model empty_model() = x = 1 - - csym = gensym() # unique per model - vn_z1 = @varname z[1] - vn_z2 = @varname z[2] - vn_z3 = @varname z[3] - vn_z4 = @varname z[4] - vn_a1 = @varname a[1] - vn_a2 = @varname a[2] - vn_b = @varname b - - vi = DynamicPPL.VarInfo() - dists = [Categorical([0.7, 0.3]), Normal()] - - # First iteration, variables are added to vi - # variables samples in order: z1,a1,z2,a2,z3 - vi = DynamicPPL.increment_num_produce!!(vi) - randr(vi, vn_z1, dists[1]) - randr(vi, vn_a1, dists[2]) - vi = DynamicPPL.increment_num_produce!!(vi) - randr(vi, vn_b, dists[2]) - randr(vi, vn_z2, dists[1]) - randr(vi, vn_a2, dists[2]) - vi = DynamicPPL.increment_num_produce!!(vi) - randr(vi, vn_z3, dists[1]) - @test DynamicPPL.getorder(vi, vn_z1) == 1 - @test DynamicPPL.getorder(vi, vn_a1) == 1 - @test DynamicPPL.getorder(vi, vn_b) == 2 - @test DynamicPPL.getorder(vi, vn_z2) == 2 - @test DynamicPPL.getorder(vi, vn_a2) == 2 - @test DynamicPPL.getorder(vi, vn_z3) == 3 - @test DynamicPPL.get_num_produce(vi) == 3 - - @test !DynamicPPL.is_flagged(vi, vn_z1, "del") - @test !DynamicPPL.is_flagged(vi, vn_a1, "del") - @test !DynamicPPL.is_flagged(vi, vn_b, "del") - @test !DynamicPPL.is_flagged(vi, vn_z2, "del") - @test !DynamicPPL.is_flagged(vi, vn_a2, "del") - @test !DynamicPPL.is_flagged(vi, vn_z3, "del") - - vi = DynamicPPL.reset_num_produce!!(vi) - vi = DynamicPPL.increment_num_produce!!(vi) - DynamicPPL.set_retained_vns_del!(vi) - @test !DynamicPPL.is_flagged(vi, vn_z1, "del") - @test !DynamicPPL.is_flagged(vi, vn_a1, "del") - @test DynamicPPL.is_flagged(vi, vn_b, "del") - @test DynamicPPL.is_flagged(vi, vn_z2, "del") - @test DynamicPPL.is_flagged(vi, vn_a2, "del") - @test DynamicPPL.is_flagged(vi, vn_z3, "del") - - vi = DynamicPPL.reset_num_produce!!(vi) - DynamicPPL.set_retained_vns_del!(vi) - @test DynamicPPL.is_flagged(vi, vn_z1, "del") - @test DynamicPPL.is_flagged(vi, vn_a1, "del") - @test DynamicPPL.is_flagged(vi, vn_b, "del") - @test DynamicPPL.is_flagged(vi, vn_z2, "del") - @test DynamicPPL.is_flagged(vi, vn_a2, "del") - @test DynamicPPL.is_flagged(vi, vn_z3, "del") - - vi = DynamicPPL.increment_num_produce!!(vi) - randr(vi, vn_z1, dists[1]) - randr(vi, vn_a1, dists[2]) - vi = DynamicPPL.increment_num_produce!!(vi) - randr(vi, vn_z2, dists[1]) - vi = DynamicPPL.increment_num_produce!!(vi) - randr(vi, vn_z3, dists[1]) - randr(vi, vn_a2, dists[2]) - @test DynamicPPL.getorder(vi, vn_z1) == 1 - @test DynamicPPL.getorder(vi, vn_a1) == 1 - @test DynamicPPL.getorder(vi, vn_b) == 2 - @test DynamicPPL.getorder(vi, vn_z2) == 2 - @test DynamicPPL.getorder(vi, vn_z3) == 3 - @test DynamicPPL.getorder(vi, vn_a2) == 3 - @test DynamicPPL.get_num_produce(vi) == 3 - - vi = empty!!(DynamicPPL.typed_varinfo(vi)) - # First iteration, variables are added to vi - # variables samples in order: z1,a1,z2,a2,z3 - vi = DynamicPPL.increment_num_produce!!(vi) - randr(vi, vn_z1, dists[1]) - randr(vi, vn_a1, dists[2]) - vi = DynamicPPL.increment_num_produce!!(vi) - randr(vi, vn_b, dists[2]) - randr(vi, vn_z2, dists[1]) - randr(vi, vn_a2, dists[2]) - vi = DynamicPPL.increment_num_produce!!(vi) - randr(vi, vn_z3, dists[1]) - @test DynamicPPL.getorder(vi, vn_z1) == 1 - @test DynamicPPL.getorder(vi, vn_z2) == 2 - @test DynamicPPL.getorder(vi, vn_z3) == 3 - @test DynamicPPL.getorder(vi, vn_a1) == 1 - @test DynamicPPL.getorder(vi, vn_a2) == 2 - @test DynamicPPL.getorder(vi, vn_b) == 2 - @test DynamicPPL.get_num_produce(vi) == 3 - - vi = DynamicPPL.reset_num_produce!!(vi) - DynamicPPL.set_retained_vns_del!(vi) - @test DynamicPPL.is_flagged(vi, vn_z1, "del") - @test DynamicPPL.is_flagged(vi, vn_a1, "del") - @test DynamicPPL.is_flagged(vi, vn_z2, "del") - @test DynamicPPL.is_flagged(vi, vn_a2, "del") - @test DynamicPPL.is_flagged(vi, vn_z3, "del") - - vi = DynamicPPL.increment_num_produce!!(vi) - randr(vi, vn_z1, dists[1]) - randr(vi, vn_a1, dists[2]) - vi = DynamicPPL.increment_num_produce!!(vi) - randr(vi, vn_z2, dists[1]) - vi = DynamicPPL.increment_num_produce!!(vi) - randr(vi, vn_z3, dists[1]) - randr(vi, vn_a2, dists[2]) - @test DynamicPPL.getorder(vi, vn_z1) == 1 - @test DynamicPPL.getorder(vi, vn_z2) == 2 - @test DynamicPPL.getorder(vi, vn_z3) == 3 - @test DynamicPPL.getorder(vi, vn_a1) == 1 - @test DynamicPPL.getorder(vi, vn_a2) == 3 - @test DynamicPPL.getorder(vi, vn_b) == 2 - @test DynamicPPL.get_num_produce(vi) == 3 - end - @testset "issue #842" begin model = DynamicPPL.TestUtils.DEMO_MODELS[1] varinfo = VarInfo(model)