diff --git a/HISTORY.md b/HISTORY.md index d67afcbfe..aaa5ac1eb 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -50,6 +50,10 @@ Other functions such as `tilde_assume` and `assume` (and their `observe` counter Note that this was effectively already the case in DynamicPPL 0.37 (where they were just wrappers around each other). The separation of these functions was primarily implemented to avoid performing extra work where unneeded (e.g. to not calculate the log-likelihood when `PriorContext` was being used). This functionality has since been replaced with accumulators (see the 0.37 changelog for more details). +### Removal of the `"del"` flag + +Previously `VarInfo` (or more correctly, the `Metadata` object within a `VarInfo`), had a flag called `"del"` for all variables. If it was set to `true` the variable was to be overwritten with a new value at the next evaluation. The new `InitContext` and related changes above make this flag unnecessary, and it has been removed. + **Other changes** ### Reimplementation of functions using `InitContext` diff --git a/src/threadsafe.jl b/src/threadsafe.jl index 6ca3b9852..f89a562e3 100644 --- a/src/threadsafe.jl +++ b/src/threadsafe.jl @@ -185,10 +185,8 @@ end values_as(vi::ThreadSafeVarInfo) = values_as(vi.varinfo) values_as(vi::ThreadSafeVarInfo, ::Type{T}) where {T} = values_as(vi.varinfo, T) -function unset_flag!( - vi::ThreadSafeVarInfo, vn::VarName, flag::String, ignoreable::Bool=false -) - return unset_flag!(vi.varinfo, vn, flag, ignoreable) +function unset_flag!(vi::ThreadSafeVarInfo, vn::VarName, flag::String) + return unset_flag!(vi.varinfo, vn, flag) end function is_flagged(vi::ThreadSafeVarInfo, vn::VarName, flag::String) return is_flagged(vi.varinfo, vn, flag) diff --git a/src/varinfo.jl b/src/varinfo.jl index 081f65ea1..062cc236b 100644 --- a/src/varinfo.jl +++ b/src/varinfo.jl @@ -423,7 +423,6 @@ Construct an empty type unstable instance of `Metadata`. function Metadata() vals = Vector{Real}() flags = Dict{String,BitVector}() - flags["del"] = BitVector() flags["trans"] = BitVector() return Metadata( @@ -887,12 +886,7 @@ function set_flag!(md::Metadata, vn::VarName, flag::String) end function set_flag!(vnv::VarNamedVector, ::VarName, flag::String) - if flag == "del" - # The "del" flag is effectively always set for a VarNamedVector, so this is a no-op. - else - throw(ErrorException("Flag $flag not valid for VarNamedVector")) - end - return vnv + throw(ErrorException("VarNamedVector does not support flags; Tried to set $(flag).")) end #### @@ -1710,7 +1704,7 @@ function BangBang.push!!(vi::VarInfo, vn::VarName, r, dist::Distribution) [1:length(val)], val, [dist], - Dict{String,BitVector}("trans" => [false], "del" => [false]), + Dict{String,BitVector}("trans" => [false]), ) vi = Accessors.@set vi.metadata[sym] = md else @@ -1744,7 +1738,6 @@ function Base.push!(meta::Metadata, vn, r, dist) push!(meta.ranges, (l + 1):(l + n)) append!(meta.vals, val) push!(meta.dists, dist) - push!(meta.flags["del"], false) push!(meta.flags["trans"], false) return meta end @@ -1770,42 +1763,25 @@ function is_flagged(metadata::Metadata, vn::VarName, flag::String) return metadata.flags[flag][getidx(metadata, vn)] end function is_flagged(::VarNamedVector, ::VarName, flag::String) - if flag == "del" - return true - else - throw(ErrorException("Flag $flag not valid for VarNamedVector")) - end + throw(ErrorException("VarNamedVector does not support flags; Tried to read $(flag).")) end -# TODO(mhauru) The "ignorable" argument is a temporary hack while developing VarNamedVector, -# but still having to support the interface based on Metadata too """ - unset_flag!(vi::VarInfo, vn::VarName, flag::String, ignorable::Bool=false + unset_flag!(vi::VarInfo, vn::VarName, flag::String Set `vn`'s value for `flag` to `false` in `vi`. - -Setting some flags for some `VarInfo` types is not possible, and by default attempting to do -so will error. If `ignorable` is set to `true` then this will silently be ignored instead. """ -function unset_flag!(vi::VarInfo, vn::VarName, flag::String, ignorable::Bool=false) - unset_flag!(getmetadata(vi, vn), vn, flag, ignorable) +function unset_flag!(vi::VarInfo, vn::VarName, flag::String) + unset_flag!(getmetadata(vi, vn), vn, flag) return vi end -function unset_flag!(metadata::Metadata, vn::VarName, flag::String, ignorable::Bool=false) +function unset_flag!(metadata::Metadata, vn::VarName, flag::String) metadata.flags[flag][getidx(metadata, vn)] = false return metadata end -function unset_flag!(vnv::VarNamedVector, ::VarName, flag::String, ignorable::Bool=false) - if ignorable - return vnv - end - if flag == "del" - throw(ErrorException("The \"del\" flag cannot be unset for VarNamedVector")) - else - throw(ErrorException("Flag $flag not valid for VarNamedVector")) - end - return vnv +function unset_flag!(vnv::VarNamedVector, ::VarName, flag::String) + throw(ErrorException("VarNamedVector does not support flags; Tried to unset $(flag).")) end # TODO: Maybe rename or something? diff --git a/test/varinfo.jl b/test/varinfo.jl index 75d8e062b..dc09ff8da 100644 --- a/test/varinfo.jl +++ b/test/varinfo.jl @@ -22,11 +22,6 @@ function randr(vi::DynamicPPL.VarInfo, vn::VarName, dist::Distribution) r = rand(dist) push!!(vi, vn, r, dist) r - elseif DynamicPPL.is_flagged(vi, vn, "del") - DynamicPPL.unset_flag!(vi, vn, "del") - r = rand(dist) - vi[vn] = DynamicPPL.tovec(r) - r else vi[vn] end @@ -300,14 +295,14 @@ end push!!(vi, vn_x, r, dist) - # del is set by default - @test !is_flagged(vi, vn_x, "del") + # trans is set by default + @test !is_flagged(vi, vn_x, "trans") - set_flag!(vi, vn_x, "del") - @test is_flagged(vi, vn_x, "del") + set_flag!(vi, vn_x, "trans") + @test is_flagged(vi, vn_x, "trans") - unset_flag!(vi, vn_x, "del") - @test !is_flagged(vi, vn_x, "del") + unset_flag!(vi, vn_x, "trans") + @test !is_flagged(vi, vn_x, "trans") end vi = VarInfo() test_varinfo!(vi)