-
Notifications
You must be signed in to change notification settings - Fork 39
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.)