Skip to content

Commit

Permalink
Attempt at implementation of VarNamedVector (Metadata alternative) (
Browse files Browse the repository at this point in the history
#555)

* initial implementation of VarNameVector

* added some hacky getval and getdist get things to work for VarInfo

* Apply suggestions from code review

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* added arbitrary metadata field as discussed

* renamed idcs to varname_to_index

* renamed vns to varnames for VarNameVector

* added keys impl for Metadata

* added push! and update! for VarNameVector

* added getindex_raw! and setindex_raw! for VarNameVector

* added `iterate` and `convert` (for `AbstractDict) impls for `VarNameVector`

* make the key and eltype part of the `VarNameVector` type

* added more tests for VarNameVector

* formatting

* more testing for VarNameVector

* minor changes to some comments

* added a bunch more tests for VarNameVector + several bugfixes in the process

* formatting

* added `similar` implementation for `VarNameVector`

* formatting

* removed debug statement

* made VarInfo slighly more generic wrt. underlying metadata

* fixed incorrect behavior in `keys` for `Metadata`

* minor style changes to VarNameVector tests

* style

* added testing of `update!` with smaller sizes and fixed bug related to this

* formatting

* move functionality related to `push!` for `VarNameVector` into `push!`

* Update src/varnamevector.jl

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* several fixes to make sampling with VarNameVector + initiall tests for
sampling with VarNameVector

* VarInfo + VarNameVector tests for all demo models

* Apply suggestions from code review

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* added docs on the design of `VarNameVector`

* Apply suggestions from code review

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* added note on `update!`

* further elaboration of the design of `VarInfo` and `VarNameVector`

* more writing improvements

* added docstring to `has_inactive_ranges` and `inactive_ranges_sweep!`

* moved docs on `VarInfo` design to a separate internals section

* writing improvements for internal docs

* further motivation of the design choices made in `VarNameVector`

* improved writing

* VarNameVector is now grown as much as needed

* updated `delete!`

* Significant changes to implementation of `VarNameVector`:
- "delete-by-mark" is now replaced by proper deletion.
- `inactive_ranges` replaced by `num_inactive`, which only keeps track
of the number of inactive entries for a given `VarName.
- `VarNameVector` is now a "grow-as-needed" structure where the
underlying also mimics the order that the user experiences.`

* added `copy` when constructing `VectorVarInfo` from `VarInfo`

* added missing `isempty` impl

* remove impl of `iterate` and instead implemented `pairs` and `values` iterators

* added missing `empty!` for `num_inactive`

* removed redundant `shift_left!` methd

* fixed `delete!` for `VarNameVector`

* added `is_contiguous` as an alterantive to `!has_inactive`

* updates to internal docs

* renamed `sweep_inactive_ranges!` to `contiguify!`

* improvements to internal docs

* more improvements to internal docs

* moved additional methods description in internals to earlier in the doc

* moved internals docs to a separate directory and split into files

* more improvements to internals doc

* formatting

* added tests for `delete!` and fixed reference to old method

* addition to `delete!` test

* added `values_as` impls for `VarNameVector`

* added docs for `replace_valus` and `values_as` for `VarNameVector`

* fixed doctest

* formatting

* temporarily disable doctests so we can build docs

* added missing compat entry for ForwardDiff in docs

* moved some shared code into methods to make things a bit cleaner

* added impl of `merge` for `VarNameVector`

* renamed a few variables in `merge` impl for `VarNameVector`

* forgot to include some changes in previous commit

* added impl of `subset` for `VarNameVector`

* fixed `pairs` impl for `VarNameVector`

* added missing impl of `subset` for `VectorVarInfo`

* added missing impl of `merge_metadata` for `VarNameVector`

* added a bunch of `from_vec_transform` and `tovec` impls to make
`VarNameVector` work with `Cholesky`, etc.

* make default args use `from_vec_transform` rather than `FromVec`

* fixed `values_as` fro `VarInfo` with `VarNameVector` as `metadata`

* fixed impl of `getindex_raw` when using integer index for `VarNameVector`

* added tests for `getindex` with `Int` index for `VarNameVector`

* fix for `setindex!` and `setindex_raw!` for `VarNameVector`

* introduction of `from_vec_transform` and `tovec` and its usage in `VarInfo`

* moved definition of `is_splat_symbol` to the file where it's used

* added `VarInfo` constructor with vector input for `VectorVarInfo`

* make `extract_priors` take the `rng` as an argument

* added `replace_values` for `Metadata`

* make link and invlink act on the `metadata` field for `VarInfo` +
implementations of these for `Metadata` and `VarNameVector`

* added temporary defs of `with_logabsdet_jacobian` and `inverse` for
`transpose` and `Bijectors.vec_to_triu`

* added invlink_with_logpdf overload for `ThreadSafeVarInfo`

* added `is_transformed` field to `VarNameVector`

* removed unnecessary defintions of `with_logabsdet_jacobian` and
`inverse` for `transpose`

* fixed issue where we were storing the wrong transformations in `VarNameVector`

* make sure `extract_priors` doesn't mutate the `varinfo`

* updated `similar` for `VarNameVector` and fixed `invlink` for `VarNameVector`

* added handling of `is_transformed` in `merge` for `VarNameVector`

* removed unnecesasry `deepcopy` from outer `link`

* updated `push!` to also `push!` on `is_transformed`

* skip tests for mutating linking when using VarNameVector

* use same projection for `Cholesky` in `VarNameVector` as in `VarInfo`

* fixed `settrans!!` for `VarInfo` with `VarNameVector`

* fixed bug in `set_flag!`

* fixed another typo

* fixed return values of `settrans!!`

* updated static transformation tests

* Update test/simple_varinfo.jl

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* removed unnecessary impl of `extract_priors`

* make `short_varinfo_name` of `TypedVarInfo` a bit more informative

* moved impl of `has_varnamevector` for `ThreadSafeVarInfo`

* added back `extract_priors` impl as we do need it

* forgot to include tests for `VarNameVector` in `runtests.jl`

* fix for `relax_container_types` in `test/varnamevector.jl`

* fixed `need_transforms_relaxation`

* updated some tests to not refer directly to `FromVec`

* introduce `from_internal_transform` and its siblings

* remove `with_logabsdet_jacobian_and_reconstruct` in favour of
`with_logabsdet_jacobian` with `from_linked_internal_transform`, etc.

* added `internal_to_linked_internal_transform` + fixed a few bugs in
the linking as a resultt

* added `linked_internal_to_internal_transform` as a complement to `interanl_to_linked_interanl_transform`

* fixed bugs in `invlink` for `VarInfo` using `linked_internal_to_internal_transform`

* more work on removing calls to `reconstruct`

* removed redundant comment

* added `from_linked_vec_transform` specialization for `LKJ`

* more work on removing references to `reconstruct`

* added `copy` in `values_from_metadata` to preserve behavior and avoid
refs to internal representation

* remove `reconstruct_and_link` and `invlink_and_reconstruct`

* replaced references to `link_and_reconstruct` and `invlink_and_reconstruct`

* introduced `recombine` and replaced calls to `reconstruct` with `n` samples

* completely removed `reconstruct`

* renamed `maybe_reconstruct_and_link` to `to_maybe_linked_internal` and
`maybe_invlink_and_reconstruct` to `from_maybe_linked_internal`

* added impls of `from_*_internal_transform` for `ThreadSafeVarInfo`

* removed `reconstruct` from docs and from exports

* renamed `getval` to `getindex_internal` and made `dist` an optional
argument for all the transform-related methods

* updated docs + added description of how internals of transforms work

* added a bunch of illustrations for the transforms docs + dot files used to generated

* temporarily removed `VarNameVector` completely

* formatting

* Update docs/src/internals/transformations.md

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Update docs/src/internals/transformations.md

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* removed refs to VectorVarInfo

* added impls of `from_internal_transform` for `ThreadSafeVarInfo`

* reverted accidental removal of old `VarInfo` constructor

* fixed incorrect `recombine` call

* removed undefined refs to `VarNameVector` stuff in `setup_varinfos`

* bump minior version because Turing breaks

* fix: was using `from_linked_internal_transform` in
`from_internal_transform` for `ThreadSafeVarInfo`

* removed `getindex_raw`

* removed redundant docstrings

* fixed tests

* fixed comparisons in tests

* try relative references for images in transformation docs

* another attempt at fixing asset-references

* fixed getindex diagrams in docs

* minor changes to comments

* remove Combinatorics as a test dep, as it's not needed for this PR

* reverted unnecessary change

* disable type-stability tests for models on older Julia versions

* removed seemingly completely unused impl of `setval!`

* Revert "temporarily removed `VarNameVector` completely"

This reverts commit 95dc8e3.

* Revert "remove Combinatorics as a test dep, as it's not needed for this PR"

This reverts commit 071bebf.

* More work on `VarNameVector` (#637)

* Update test/model.jl

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Markus Hauru <[email protected]>

* Type-stability tests are now correctly using `rand_prior_true` instead
of `rand`

* `getindex_internal` now calls `getindex` instead of `view`, as the
latter can result in type-instability since transformed variables
typically result in non-view even if input is a view

* Removed seemingly unnecessary definition of `getindex_internal`

* Fixed references to `newmetadata` which has been replaced by `replace_values`

* Made implementation of `recombine` more explicit

* Added docstrings for `untyped_varinfo` and `typed_varinfo`

* Added TODO comment about implementing `view` for `VarInfo`

* Fixed potential infinite recursion as suggested by @mhauru

* added docstring to `from_vec_trnasform_for_size

* Replaced references to `vectorize(dist, x)` with `tovec(x)`

* Fixed docstring

* Update src/extract_priors.jl

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Bump minor version since this is a breaking change

* Apply suggestions from code review

Co-authored-by: Markus Hauru <[email protected]>

* Update src/varinfo.jl

Co-authored-by: Tor Erlend Fjelde <[email protected]>

* Apply suggestions from code review

* Apply suggestions from code review

* Update src/extract_priors.jl

Co-authored-by: Xianda Sun <[email protected]>

* Added fix for product distributions of targets with changing support + tests

* Addeed tests for product of distributions with dynamic support

* Apply suggestions from code review

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Fix typos, improve docstrings

* Use Accessors rather than Setfield

* Simplify group_by_symbol

* Add short_varinfo_name(::VectorVarInfo)

* Add tests for subset

* Export VectorVarInfo

* Tighter type bound for has_varnamevector

* Add some VectorVarName methods

* Add todo notes, remove dead code, fix a typo.

* Bug fixes and small improvements

* VarNameVector improvements

* Improve generated_quantities and its tests

* Improvement to VarNameVector

* Fix a test to work with VectorVarName

* Fix generated_quantities

* Fix type stability issues

* Various VarNameVector fixes and improvements

* Bump version number

* Improvements to generated_quantities

* Code formatting

* Code style

* Add fallback implementation of findinds for VarNameVector

* Rename VarNameVector to VarNamedVector

* More renaming of VNV. Remove unused VarNamedVector.metadata field.

* Rename FromVec to ReshapeTransform

* Progress towards having VarNamedVector as storage for SimpleVarInfo

* Fix unflatten(vnv::VarNamedVector, vals)

* More work on SimpleVarInfo{VarNamedVector}

* More tests for SimpleVarInfo{VarNamedVector}

* More tests for SimpleVarInfo{VarNamedVector}

* Respond to review feedback

* Add float_type_with_fallback(::Type{Union{}})

* Move some VNV functions to the correct file

* Fix push! for VNV

* Rename VNV.is_transformed to VNV.is_unconstrained

* Improve VNV docstring

* Add VNV inner constructor checks

* Reorganise parts of VNV code

* Documentation and small fixes for VNV

* Rename loosen_types!! and tighten_types, add docstrings and doctests

* Rename VarNameVector to VarNamedVector in docs

* Documentation and small fixes to VNV

* Fix subset(::VarNamedVector, args...) for unconstrained variables.

---------

Co-authored-by: Tor Erlend Fjelde <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Xianda Sun <[email protected]>

* Bump Bijectors dependecy

* Remove dead TODO note

* Remove old TODOs, improve VNV invlinking

* Fix from_vec_transform for 0-dim arrays

* Fix unflatten for VarInfo

* Fix some VarInfo index getters

* Change how VNV handles transformations, and other VNV stuff

* Small docs fixes

* Small fixes all over for VNV

* Add comments

* Fix some tests

* Change long string formatting to support Julia 1.6

* Small changes to ReshapeTransformation

* Revert unrelated changes to ReverseDiff extension

* Improve VarNamedVector VarInfo testing

* Fix some depwarns

* Improvements to test/simple_varinfo.jl

* Fix for unset_flag!, better docstring

* Add a comment about hasvalue/getvalue

* Add @non_differentiable calls to work around Zygote limitations

* Fix docs, workaround Zygote issue

* Remove outdated workaround

* Move has_varnamedvector(varinfo::VarInfo) to abstract_varinfo.jl

* Make copies of logp and num_produce in subset

* Rename getindex_raw to getindex_internal

* Add push!(::VarNamedVector, ::Pair)

* Improve VarNamedVector docs

* Simplify VarNamedVector constructors

* Change how VNV setindex! et al work

* More improvements to VNV setters and their tests

* Fix style issues in VNV

* Update VNV docs. Add haskey to VarInfo

* Fix VarInfo docs

* Disable a test that only works for VectorVarInfo

* Fix bug in isempty(::TypedVarInfo)

* Make some doctests platform independent

* Better implementation of haskey(::VarInfo, ::VarName)

Co-authored-by: Tor Erlend Fjelde <[email protected]>

* Improve haskey for VarInfo

* Make a VNV doctest more robust

* Remote IndexStyle for VNV

* Clean up an old comment

* Fix haskey(::VarInfo, ::VarName)

* Clarify a TODO note in varinfo.jl

* Reintroduce Int indexing to VNV

* Stop exporting any VNV stuff

* Fix docs

* Revert default VarInfo metadata type to Metadata

* Fix a few trivial issues with Metadata

* Docs bug fix

* Fix type constraint

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Markus Hauru <[email protected]>
Co-authored-by: Markus Hauru <[email protected]>
Co-authored-by: Xianda Sun <[email protected]>
  • Loading branch information
5 people authored Oct 8, 2024
1 parent 7f91c07 commit c38e65f
Show file tree
Hide file tree
Showing 25 changed files with 3,339 additions and 262 deletions.
4 changes: 2 additions & 2 deletions Project.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name = "DynamicPPL"
uuid = "366bfd00-2699-11ea-058f-f148b4cae6d8"
version = "0.29.2"
version = "0.30"

[deps]
ADTypes = "47edcb42-4c32-4615-8424-f2b9edc5f35b"
Expand Down Expand Up @@ -46,7 +46,7 @@ AbstractMCMC = "5"
AbstractPPL = "0.8.4, 0.9"
Accessors = "0.1"
BangBang = "0.4.1"
Bijectors = "0.13.9"
Bijectors = "0.13.18"
ChainRulesCore = "1"
Compat = "4"
ConstructionBase = "1.5.4"
Expand Down
2 changes: 1 addition & 1 deletion docs/make.jl
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ makedocs(;
pages=[
"Home" => "index.md",
"API" => "api.md",
"Internals" => ["internals/transformations.md"],
"Internals" => ["internals/varinfo.md", "internals/transformations.md"],
],
checkdocs=:exports,
doctest=false,
Expand Down
11 changes: 10 additions & 1 deletion docs/src/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -294,10 +294,19 @@ resetlogp!!
```@docs
keys
getindex
DynamicPPL.getindex_internal
push!!
empty!!
isempty
DynamicPPL.getindex_internal
DynamicPPL.setindex_internal!
DynamicPPL.update_internal!
DynamicPPL.insert_internal!
DynamicPPL.length_internal
DynamicPPL.reset!
DynamicPPL.update!
DynamicPPL.insert!
DynamicPPL.loosen_types!!
DynamicPPL.tighten_types
```

```@docs
Expand Down
302 changes: 302 additions & 0 deletions docs/src/internals/varinfo.md

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions ext/DynamicPPLChainRulesCoreExt.jl
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,6 @@ ChainRulesCore.@non_differentiable DynamicPPL.updategid!(
# No need + causes issues for some AD backends, e.g. Zygote.
ChainRulesCore.@non_differentiable DynamicPPL.infer_nested_eltype(x)

ChainRulesCore.@non_differentiable DynamicPPL.recontiguify_ranges!(ranges)

end # module
143 changes: 137 additions & 6 deletions ext/DynamicPPLMCMCChainsExt.jl
Original file line number Diff line number Diff line change
Expand Up @@ -42,21 +42,152 @@ function DynamicPPL.varnames(c::MCMCChains.Chains)
return keys(c.info.varname_to_symbol)
end

"""
generated_quantities(model::Model, chain::MCMCChains.Chains)
Execute `model` for each of the samples in `chain` and return an array of the values
returned by the `model` for each sample.
# Examples
## General
Often you might have additional quantities computed inside the model that you want to
inspect, e.g.
```julia
@model function demo(x)
# sample and observe
θ ~ Prior()
x ~ Likelihood()
return interesting_quantity(θ, x)
end
m = demo(data)
chain = sample(m, alg, n)
# To inspect the `interesting_quantity(θ, x)` where `θ` is replaced by samples
# from the posterior/`chain`:
generated_quantities(m, chain) # <= results in a `Vector` of returned values
# from `interesting_quantity(θ, x)`
```
## Concrete (and simple)
```julia
julia> using DynamicPPL, Turing
julia> @model function demo(xs)
s ~ InverseGamma(2, 3)
m_shifted ~ Normal(10, √s)
m = m_shifted - 10
for i in eachindex(xs)
xs[i] ~ Normal(m, √s)
end
return (m, )
end
demo (generic function with 1 method)
julia> model = demo(randn(10));
julia> chain = sample(model, MH(), 10);
julia> generated_quantities(model, chain)
10×1 Array{Tuple{Float64},2}:
(2.1964758025119338,)
(2.1964758025119338,)
(0.09270081916291417,)
(0.09270081916291417,)
(0.09270081916291417,)
(0.09270081916291417,)
(0.09270081916291417,)
(0.043088571494005024,)
(-0.16489786710222099,)
(-0.16489786710222099,)
```
"""
function DynamicPPL.generated_quantities(
model::DynamicPPL.Model, chain_full::MCMCChains.Chains
)
chain = MCMCChains.get_sections(chain_full, :parameters)
varinfo = DynamicPPL.VarInfo(model)
iters = Iterators.product(1:size(chain, 1), 1:size(chain, 3))
return map(iters) do (sample_idx, chain_idx)
# Update the varinfo with the current sample and make variables not present in `chain`
# to be sampled.
DynamicPPL.setval_and_resample!(varinfo, chain, sample_idx, chain_idx)
if DynamicPPL.supports_varname_indexing(chain)
varname_pairs = _varname_pairs_with_varname_indexing(
chain, varinfo, sample_idx, chain_idx
)
else
varname_pairs = _varname_pairs_without_varname_indexing(
chain, varinfo, sample_idx, chain_idx
)
end
fixed_model = DynamicPPL.fix(model, Dict(varname_pairs))
return fixed_model()
end
end

"""
_varname_pairs_with_varname_indexing(
chain::MCMCChains.Chains, varinfo, sample_idx, chain_idx
)
# TODO: Some of the variables can be a view into the `varinfo`, so we need to
# `deepcopy` the `varinfo` before passing it to `model`.
model(deepcopy(varinfo))
Get pairs of `VarName => value` for all the variables in the `varinfo`, picking the values
from the chain.
This implementation assumes `chain` can be indexed using variable names, and is the
preffered implementation.
"""
function _varname_pairs_with_varname_indexing(
chain::MCMCChains.Chains, varinfo, sample_idx, chain_idx
)
vns = DynamicPPL.varnames(chain)
vn_parents = Iterators.map(vns) do vn
# The call nested_setindex_maybe! is used to handle cases where vn is not
# the variable name used in the model, but rather subsumed by one. Except
# for the subsumption part, this could be
# vn => getindex_varname(chain, sample_idx, vn, chain_idx)
# TODO(mhauru) This call to nested_setindex_maybe! is unintuitive.
DynamicPPL.nested_setindex_maybe!(
varinfo, DynamicPPL.getindex_varname(chain, sample_idx, vn, chain_idx), vn
)
end
varname_pairs = Iterators.map(Iterators.filter(!isnothing, vn_parents)) do vn_parent
vn_parent => varinfo[vn_parent]
end
return varname_pairs
end

"""
Check which keys in `key_strings` are subsumed by `vn_string` and return the their values.
The subsumption check is done with `DynamicPPL.subsumes_string`, which is quite weak, and
won't catch all cases. We should get rid of this if we can.
"""
# TODO(mhauru) See docstring above.
function _vcat_subsumed_values(vn_string, values, key_strings)
indices = findall(Base.Fix1(DynamicPPL.subsumes_string, vn_string), key_strings)
return !isempty(indices) ? reduce(vcat, values[indices]) : nothing
end

"""
_varname_pairs_without_varname_indexing(
chain::MCMCChains.Chains, varinfo, sample_idx, chain_idx
)
Get pairs of `VarName => value` for all the variables in the `varinfo`, picking the values
from the chain.
This implementation does not assume that `chain` can be indexed using variable names. It is
thus not guaranteed to work in cases where the variable names have complex subsumption
patterns, such as if the model has a variable `x` but the chain stores `x.a[1]`.
"""
function _varname_pairs_without_varname_indexing(
chain::MCMCChains.Chains, varinfo, sample_idx, chain_idx
)
values = chain.value[sample_idx, :, chain_idx]
keys = Base.keys(chain)
keys_strings = map(string, keys)
varname_pairs = [
vn => _vcat_subsumed_values(string(vn), values, keys_strings) for
vn in Base.keys(varinfo)
]
return varname_pairs
end

end
1 change: 1 addition & 0 deletions src/DynamicPPL.jl
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ include("sampler.jl")
include("varname.jl")
include("distribution_wrappers.jl")
include("contexts.jl")
include("varnamedvector.jl")
include("abstract_varinfo.jl")
include("threadsafe.jl")
include("varinfo.jl")
Expand Down
17 changes: 12 additions & 5 deletions src/abstract_varinfo.jl
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ julia> # Just use an example model to construct the `VarInfo` because we're lazy
julia> vi[@varname(s)] = 1.0; vi[@varname(m)] = 2.0;
julia> # For the sake of brevity, let's just check the type.
md = values_as(vi); md.s isa DynamicPPL.Metadata
md = values_as(vi); md.s isa Union{DynamicPPL.Metadata, DynamicPPL.VarNamedVector}
true
julia> values_as(vi, NamedTuple)
Expand All @@ -321,7 +321,7 @@ julia> # Just use an example model to construct the `VarInfo` because we're lazy
julia> vi[@varname(s)] = 1.0; vi[@varname(m)] = 2.0;
julia> # For the sake of brevity, let's just check the type.
values_as(vi) isa DynamicPPL.Metadata
values_as(vi) isa Union{DynamicPPL.Metadata, Vector}
true
julia> values_as(vi, NamedTuple)
Expand Down Expand Up @@ -349,7 +349,7 @@ Determine the default `eltype` of the values returned by `vi[spl]`.
This should generally not be called explicitly, as it's only used in
[`matchingvalue`](@ref) to determine the default type to use in place of
type-parameters passed to the model.
This method is considered legacy, and is likely to be deprecated in the future.
"""
function Base.eltype(vi::AbstractVarInfo, spl::Union{AbstractSampler,SampleFromPrior})
Expand All @@ -363,6 +363,13 @@ function Base.eltype(vi::AbstractVarInfo, spl::Union{AbstractSampler,SampleFromP
return eltype(T)
end

"""
has_varnamedvector(varinfo::VarInfo)
Returns `true` if `varinfo` uses `VarNamedVector` as metadata.
"""
has_varnamedvector(vi::AbstractVarInfo) = false

# TODO: Should relax constraints on `vns` to be `AbstractVector{<:Any}` and just try to convert
# the `eltype` to `VarName`? This might be useful when someone does `[@varname(x[1]), @varname(m)]` which
# might result in a `Vector{Any}`.
Expand Down Expand Up @@ -554,7 +561,7 @@ end
link([t::AbstractTransformation, ]vi::AbstractVarInfo, model::Model)
link([t::AbstractTransformation, ]vi::AbstractVarInfo, spl::AbstractSampler, model::Model)
Transform the variables in `vi` to their linked space without mutating `vi`, using the transformation `t`.
Transform the variables in `vi` to their linked space without mutating `vi`, using the transformation `t`.
If `t` is not provided, `default_transformation(model, vi)` will be used.
Expand All @@ -573,7 +580,7 @@ end
invlink!!([t::AbstractTransformation, ]vi::AbstractVarInfo, model::Model)
invlink!!([t::AbstractTransformation, ]vi::AbstractVarInfo, spl::AbstractSampler, model::Model)
Transform the variables in `vi` to their constrained space, using the (inverse of)
Transform the variables in `vi` to their constrained space, using the (inverse of)
transformation `t`, mutating `vi` if possible.
If `t` is not provided, `default_transformation(model, vi)` will be used.
Expand Down
17 changes: 14 additions & 3 deletions src/context_implementations.jl
Original file line number Diff line number Diff line change
Expand Up @@ -240,9 +240,14 @@ function assume(
if haskey(vi, vn)
# Always overwrite the parameters with new ones for `SampleFromUniform`.
if sampler isa SampleFromUniform || is_flagged(vi, vn, "del")
unset_flag!(vi, vn, "del")
# TODO(mhauru) Is it important to unset the flag here? The `true` allows us
# to ignore the fact that for VarNamedVector this does nothing, but I'm unsure
# if that's okay.
unset_flag!(vi, vn, "del", true)
r = init(rng, dist, sampler)
f = to_maybe_linked_internal_transform(vi, vn, dist)
# TODO(mhauru) This should probably be call a function called setindex_internal!
# Also, if we use !! we shouldn't ignore the return value.
BangBang.setindex!!(vi, f(r), vn)
setorder!(vi, vn, get_num_produce(vi))
else
Expand Down Expand Up @@ -516,7 +521,10 @@ function get_and_set_val!(
if haskey(vi, vns[1])
# Always overwrite the parameters with new ones for `SampleFromUniform`.
if spl isa SampleFromUniform || is_flagged(vi, vns[1], "del")
unset_flag!(vi, vns[1], "del")
# TODO(mhauru) Is it important to unset the flag here? The `true` allows us
# to ignore the fact that for VarNamedVector this does nothing, but I'm unsure if
# that's okay.
unset_flag!(vi, vns[1], "del", true)
r = init(rng, dist, spl, n)
for i in 1:n
vn = vns[i]
Expand Down Expand Up @@ -554,7 +562,10 @@ function get_and_set_val!(
if haskey(vi, vns[1])
# Always overwrite the parameters with new ones for `SampleFromUniform`.
if spl isa SampleFromUniform || is_flagged(vi, vns[1], "del")
unset_flag!(vi, vns[1], "del")
# TODO(mhauru) Is it important to unset the flag here? The `true` allows us
# to ignore the fact that for VarNamedVector this does nothing, but I'm unsure if
# that's okay.
unset_flag!(vi, vns[1], "del", true)
f = (vn, dist) -> init(rng, dist, spl)
r = f.(vns, dists)
for i in eachindex(vns)
Expand Down
Loading

3 comments on commit c38e65f

@penelopeysm
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@torfjelde @mhauru @sunxd3 Did you all want to create a release for this? Just asking as I'm not sure if I should bump the version on another PR.

@mhauru
Copy link
Member

@mhauru mhauru commented on c38e65f Oct 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, let's.

@JuliaRegistrator register

@JuliaRegistrator
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Registration pull request created: JuliaRegistries/General/117074

Tip: Release Notes

Did you know you can add release notes too? Just add markdown formatted text underneath the comment after the text
"Release notes:" and it will be added to the registry PR, and if TagBot is installed it will also be added to the
release that TagBot creates. i.e.

@JuliaRegistrator register

Release notes:

## Breaking changes

- blah

To add them here just re-invoke and the PR will be updated.

Tagging

After the above pull request is merged, it is recommended that a tag is created on this repository for the registered package version.

This will be done automatically if the Julia TagBot GitHub Action is installed, or can be done manually through the github interface, or via:

git tag -a v0.30.0 -m "<description of version>" c38e65f3465f123a5147e2ddb0f49cc952487c0b
git push origin v0.30.0

Please sign in to comment.