TuringLang / TuringLang/DynamicPPL.jl
VNT: Generalising PartialArrays
Nobody has claimed this yet.
- Dominant language
- Julia
- Stars
- 286
- Forks
- 41
- Avg merge
- 1d 11h
- Merged PRs (30d)
- 34
Description
PartialArrays work fine ... as long as you're trying to represent an Array.
Other things that you can index into will break, Dicts for example. This is on current breaking; it would have worked on v0.39.
julia> using DynamicPPL, Distributions
Precompiling DynamicPPL...
1 dependency successfully precompiled in 5 seconds. 123 already precompiled.
julia> using DynamicPPL, Distributions
julia> @model function f()
x = Dict()
x["a"] ~ Normal()
end
f (generic function with 2 methods)
julia> f()()
ERROR: ArgumentError: Attempted to set a value with an index of a, but no template was provided for the array. For such an index, a template is needed to determine the shape and type of the array so that the indexed data can be stored correctly.
Stacktrace:
[1] largest_index(x::String)
@ DynamicPPL.VarNamedTuples ~/ppl/dppl/src/varnamedtuple/partial_array.jl:129
[2] map
@ ./tuple.jl:355 [inlined]
[3] get_maximum_size_from_indices(ix::String; kw::@Kwargs{})
@ DynamicPPL.VarNamedTuples ~/ppl/dppl/src/varnamedtuple/partial_array.jl:140
[4] get_maximum_size_from_indices(ix::String)
@ DynamicPPL.VarNamedTuples ~/ppl/dppl/src/varnamedtuple/partial_array.jl:138
[5] make_leaf_singleindex(value::VectorValue{…}, coptic::AbstractPPL.Index{…}, template::Dict{…})
@ DynamicPPL.VarNamedTuples ~/ppl/dppl/src/varnamedtuple/getset.jl:381
[6] make_leaf(value::VectorValue{…}, optic::AbstractPPL.Index{…}, template::Dict{…})
@ DynamicPPL.VarNamedTuples ~/ppl/dppl/src/varnamedtuple/getset.jl:336
[7] _setindex_optic!!(vnt::VarNamedTuple{…}, value::VectorValue{…}, optic::AbstractPPL.Property{…}, template::DynamicPPL.VarNamedTuples.SkipTemplate{…}; allow_new::Val{…})
@ DynamicPPL.VarNamedTuples ~/ppl/dppl/src/varnamedtuple/getset.jl:269
[8] _setindex_optic!!(vnt::VarNamedTuple{…}, value::VectorValue{…}, optic::AbstractPPL.Property{…}, template::DynamicPPL.VarNamedTuples.SkipTemplate{…})
@ DynamicPPL.VarNamedTuples ~/ppl/dppl/src/varnamedtuple/getset.jl:250
[9] templated_setindex!!(vnt::VarNamedTuple{…}, value::VectorValue{…}, vn::VarName{…}, template::Dict{…})
[...]
The nice thing about this, is that I believe we can fix this using a nice Julian multiple-dispatch thing. Look at the last call in the stack trace before I truncated it:
[9] templated_setindex!!(vnt::VarNamedTuple{…}, value::VectorValue{…}, vn::VarName{…}, template::Dict{…})
Contrary to what the error message (ArgumentError: Attempted to set a value with an index of a, but no template was provided for the array) claims, we do have the Dict template here. The problem is that templated_setindex!! doesn't actually know how to make use of the Dict template. I am pretty sure that in principle, all that is needed to make this work is to overload this method and make it cleverer so that it can do what it needs to do.
This is not unlike adding a new rule for an AD package.
For other behaviour to work correctly, we would probably also have to overload a series of other functions, namely _getindex_optic, _haskey_optic, _setindex_optic!!, etc. Now this seems like a faff. Why do this? I think it's good because it will allow us to:
- clean up some of the nasty code in
src/varnamedtuple/getset.jlthat is full of special cases, and instead separate each case into its own method signature - in turn that allows us to have much better understanding of what functions like
make_leafshould do, and thereby unit test them much more robustly - clearly communicate what functions must be implemented for a data structure to be implemented (for example, if we want to allow things like DataFrames, what do we need to overload)?
- (the big one) thereby clearly define what is supported and what is not supported in DynamicPPL: if a data type T implements the necessary interface (i.e. if it overloads these methods, and the result of calling these methods obeys certain properties that we can clearly express, then it is supported. Otherwise not.)
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with src/varnamedtuple/getset.jl and src/varnamedtuple/partial_array.jl, tracing templated_setindex!!, _getindex_optic, _haskey_optic, _setindex_optic!!, and the index-size helpers shown in the stack trace. Done means Dict indexing no longer fails when used in a model, and the supported data-structure interface and its required behavior are clearly defined.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- julia
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100