Error when running example on missing values #1464
Comments
|
This is really strange. I'm on Julia 1.5.1 and the above works but doesn't actually produce samples for |
|
Let me rephrase that: DynamicPPL.jl is doing the right thing, e.g. julia> # x[1] is a parameter, but x[2] is an observation
model = gdemo([missing, 2.4]);
julia> var_info = Turing.VarInfo(model);
julia> var_info.metadata.x
DynamicPPL.Metadata{Dict{DynamicPPL.VarName{:x,Tuple{Tuple{Int64}}},Int64},Array{Normal{Float64},1},Array{DynamicPPL.VarName{:x,Tuple{Tuple{Int64}}},1},Array{Float64,1},Array{Set{DynamicPPL.Selector},1}}(Dict{DynamicPPL.VarName{:x,Tuple{Tuple{Int64}}},Int64}(x[1] => 1), DynamicPPL.VarName{:x,Tuple{Tuple{Int64}}}[x[1]], UnitRange{Int64}[1:1], [-1.3576106773798338], Normal{Float64}[Normal{Float64}(μ=0.8828819868130773, σ=1.7668934766490565)], Set{DynamicPPL.Selector}[Set()], [0], Dict{String,BitArray{1}}("del" => [0],"trans" => [0]))But Turing.jl messes up when bundling the chains, it seems: julia> names(sample(model, HMC(0.01, 5), 500), :parameters)
2-element Array{Symbol,1}:
:m
:s |
|
@torfjelde What Turing version are you on? I'll try initializing a new Julia environment to match yours to see if the issue is 1.5.3 or not. |
(TestTuring) pkg> st
Project TestTuring v0.1.0
Status `/tmp/TestTuring/Project.toml`
[fce5fe82] Turing v0.14.11So should be the same as you. |
|
Excuse me, what? julia> using Turing
julia> @model function gdemo(x)
s ~ InverseGamma(2, 3)
m ~ Normal(0, sqrt(s))
for i in eachindex(x)
x[i] ~ Normal(m, sqrt(s))
end
end
gdemo (generic function with 1 method)
julia> # x[1] is a parameter, but x[2] is an observation
model = gdemo([missing, 2.4]);
julia> keys(Turing.VarInfo(model).metadata) # Yes
(:s, :m, :x)
julia> keys(Turing.VarInfo(model).metadata) # NO!
(:s, :m)
julia> keys(Turing.VarInfo(model).metadata) # NO!
(:s, :m)
julia> keys(Turing.VarInfo(model).metadata) # NO!
(:s, :m)There's something very strange going on here. |
|
Ah, think I'm on to something here: julia> model
DynamicPPL.Model{var"#1#2",(:x,),(),(),Tuple{Array{Union{Missing, Float64},1}},Tuple{}}(:gdemo, var"#1#2"(), (x = Union{Missing, Float64}[-3.0877422804870696, 2.4],), NamedTuple())It seems like we end up mutating the argument rather than a copy of it. So in subsequent calls, no elements of |
|
I am pretty sure this was correct before. I was calling |
|
Yeah it's definitively a recent introduction. |
|
Try a deepcopy here in DPPL. function matchingvalue(sampler, vi, value)
T = typeof(value)
if hasmissing(T)
return get_matching_type(sampler, vi, T)(value)
else
return value
end
end |
|
You mean like function matchingvalue(sampler, vi, value)
T = typeof(value)
if hasmissing(T)
return get_matching_type(sampler, vi, T)(value)
else
return deepcopy(value)
end
end? |
get_matching_type(sampler, vi, T)(deepcopy(value)) |
|
You beautiful man @mohamed82008 ❤️ julia> using Turing
[ Info: Precompiling Turing [fce5fe82-541a-59a6-adf8-730c64b5f9a0]
julia> @model function gdemo(x)
s ~ InverseGamma(2, 3)
m ~ Normal(0, sqrt(s))
for i in eachindex(x)
x[i] ~ Normal(m, sqrt(s))
end
end
gdemo (generic function with 1 method)
julia> # x[1] is a parameter, but x[2] is an observation
model = gdemo([missing, 2.4]);
julia> keys(Turing.VarInfo(model).metadata) # Yes
(:s, :m, :x)
julia> keys(Turing.VarInfo(model).metadata) # NO!
(:s, :m)
julia> keys(Turing.VarInfo(model).metadata) # NO!
(:s, :m)
julia> function DynamicPPL.matchingvalue(sampler, vi, value)
T = typeof(value)
if DynamicPPL.hasmissing(T)
return convert(DynamicPPL.get_matching_type(sampler, vi, T), deepcopy(value))
else
return value
end
end
julia> # x[1] is a parameter, but x[2] is an observation
model = gdemo([missing, 2.4]);
julia> keys(Turing.VarInfo(model).metadata) # Yes
(:s, :m, :x)
julia> keys(Turing.VarInfo(model).metadata) # Yes!
(:s, :m, :x)
julia> keys(Turing.VarInfo(model).metadata) # YES!
(:s, :m, :x)Now the strange thing is that the only recent change we made is this one: TuringLang/DynamicPPL.jl@8c9004e#diff-365fc3b77837a3d3be094efbadc976064a26a40534c0adfb9ca65dc98819134e. It seems like the error occured because: julia> a = [1.0, 2.0]
2-element Array{Float64,1}:
1.0
2.0
julia> T = typeof(a)
Array{Float64,1}
julia> b = T(a)
2-element Array{Float64,1}:
1.0
2.0
julia> a[1] = 3.
3.0
julia> a
2-element Array{Float64,1}:
3.0
2.0
julia> b
2-element Array{Float64,1}:
1.0
2.0versus julia> a = [1.0, 2.0]
2-element Array{Float64,1}:
1.0
2.0
julia> b = convert(typeof(a), a)
2-element Array{Float64,1}:
1.0
2.0
julia> a[1] = 3.
3.0
julia> b
2-element Array{Float64,1}:
3.0
2.0Very subtle stuff. So it's actually a bit unclear whether the change in the above commit is a good idea? @devmotion @mohamed82008 Thoughts? |
|
So @bgroenks96 if you copy-paste this: function DynamicPPL.matchingvalue(sampler, vi, value)
T = typeof(value)
if DynamicPPL.hasmissing(T)
return convert(DynamicPPL.get_matching_type(sampler, vi, T), deepcopy(value))
else
return value
end
endafter |
|
We can just revert that change and write a comment to explain this. |
|
Reverting is not a good idea: TuringLang/DynamicPPL.jl#181 |
|
Hmm I see. We can define a |
|
DynamicPPL should not depend on Libtask. |
|
Then let's convert and check if the output is |
|
Hmm it feels a bit suboptimal, doesn't it? Often it should be fine to not copy the input. |
|
No we always need to copy in the missing branch. |
|
Ah wait, this branch is just used if some |
|
BTW looking at this implementation and thinking about our desire for a simpler API: is it actually useful to have both |
|
@torfjelde Thank you! 👏 This seems to do the trick. I'm not sure why I got the error and you didn't... but the issue is resolved either way! |
We need to convert the type for AD. |
|
The suggestion was to define a default implementation in DynamicPPL and to specialize it in Turing for samplers owned by Turing if needed. Basically like currently done for |
|
This won't simplify the API, it will just change it. |
|
It's only one method where currently we have two? |
|
But only 1 of them is overloaded. The other is internal. |
This fix indeed works, I have to use it now too otherwise I get the same error in my model with missing values it seems. |
|
@mohamed82008 @devmotion Any updates on getting this fix into the official package? |
Question on: https://github.com/TuringLang/Turing.jl/edit/master/docs/_docs/using-turing/guide.md
Using the exact code from the docs:
I get this error:
So I suppose either missing value handling is broken or the docs are out of date?
Or maybe my Julia version is too new?
Turing v.0.14.11
Julia 1.5.3
The text was updated successfully, but these errors were encountered: