Different approach to how observations/missings are stored in the model #268

phipsgabler · 2021-06-21T16:06:31Z

Here's a less (but still somewhat) radical approach, in preparation to implement AbstractPPL's condition/decondition.

The most important user-facing outcome is probably this:

julia> @model function demo2(x, y, θ::Float64; zz = 1)
           x ~ Normal()
           y[1] ~ Bla()
       end
demo2 (generic function with 1 method)

julia> m = demo2(missing, [1.0], 1.0)
Model demo2 given
    constants    θ, zz
    observations x, y

julia> m2 = demo2(missing, [missing], 1.0)
Model demo2 given
    constants    θ, zz
    observations x, y

julia> DynamicPPL.isobservation(@varname(y[1]), m2)
false

julia> DynamicPPL.isobservation(@varname(y[1]), m)
true

julia> DynamicPPL.getarguments(m)
(x = missing, y = [1.0], θ = 1.0, zz = 1)

The essence of the implementation is that we can match model arguments and tildes at compile time:

julia> @macroexpand @model function demo2(x, y, θ::Float64; zz = 1)
           x ~ Normal()
           y[1] ~ Normal()
       end
quote
    $(Expr(:meta, :doc))
    function demo2(x, y, θ::Float64; zz = 1)
        #= REPL[9]:1 =#
        var"##evaluator#284" = ((__model__::Model, __varinfo__::AbstractVarInfo, __context__::DynamicPPL.AbstractContext, x, y, θ::Float64, zz)->begin
                    begin
                        #= REPL[9]:1 =#
                        #= REPL[9]:2 =#
                        begin
                            var"##vn#274" = (VarName){:x}()
                            var"##inds#275" = ()
                            var"##isobservation#276" = (DynamicPPL.isobservation)(var"##vn#274", __model__)
                            if var"##isobservation#276"
                                (DynamicPPL.tilde_observe!)(__context__, (DynamicPPL.check_tilde_rhs)(Normal()), x, var"##vn#274", var"##inds#275", __varinfo__)
                            else
                                x = (DynamicPPL.tilde_assume!)(__context__, (DynamicPPL.unwrap_right_vn)((DynamicPPL.check_tilde_rhs)(Normal()), var"##vn#274")..., var"##inds#275", __varinfo__)
                            end
                        end
                        #= REPL[9]:3 =#
                        begin
                            var"##vn#277" = (VarName){:y}(((1,),))
                            var"##inds#278" = ((1,),)
                            var"##isobservation#279" = (DynamicPPL.isobservation)(var"##vn#277", __model__)
                            if var"##isobservation#279"
                                (DynamicPPL.tilde_observe!)(__context__, (DynamicPPL.check_tilde_rhs)(Normal()), y[1], var"##vn#277", var"##inds#278", __varinfo__)
                            else
                                y[1] = (DynamicPPL.tilde_assume!)(__context__, (DynamicPPL.unwrap_right_vn)((DynamicPPL.check_tilde_rhs)(Normal()), var"##vn#277")..., var"##inds#278", __varinfo__)
                            end
                        end
                    end
                end)
        var"##x#280" = (DynamicPPL.Observation)(x)
        var"##y#281" = (DynamicPPL.Observation)(y)
        var"##θ#282" = (DynamicPPL.Constant)(θ)
        var"##zz#283" = (DynamicPPL.Constant)(zz)
        return (Model)(:demo2, var"##evaluator#284", (DynamicPPL.namedtuple)(NamedTuple{(:x, :y, :θ, :zz), Tuple{Core.Typeof(var"##x#280"), Core.Typeof(var"##y#281"), Core.Typeof(var"##θ#282"), Core.Typeof(var"##zz#283")}}, (var"##x#280", var"##y#281", var"##θ#282", var"##zz#283")))
    end
end

Notes:

I have disabled the prob macro test. There are no "default arguments" stored in the model anymore, but they are needed only to re-build the model, which should be doable in more elgant ways. I hope that the functionality of the prob macro can be replaced through AbstractPPL.
The "type tags" I use to distinguish "observed" and "constant" arguments were a rather spontaneous invention to preserve both the distinction and the order -- I think it works, but please think hard of corner cases.
I am not entirely satisfied with the fact that getobservations and isobservation are not congruent with missings present. Any idea for better naming?

phipsgabler · 2021-06-21T16:07:22Z

src/compiler.jl

-    # Generate main body
-    modelinfo[:body] = generate_mainbody(mod, modelinfo[:modeldef][:body], warn)
+    # Generate main body and find all variable symbols
+    modelinfo[:body], modelinfo[:varnames] = generate_mainbody(


Should generate_mainbody get a new name? I figured there is no need to traverse the expression twice, so I just added the varname extraction here...

src/compiler.jl

src/model.jl

src/varname.jl

devmotion · 2021-06-21T16:53:19Z

I am confused, isn't this wrong? Missing values are no observations but have to be sampled.

julia> m = demo2(missing, [1.0], 1.0)
Model demo2 given
    constants    θ, zz
    observations x, y

julia> m2 = demo2(missing, [missing], 1.0)
Model demo2 given
    constants    θ, zz
    observations x, y

It's a bit difficult to parse all changes, it seems e.g. getargnames is just renamed but there is no new or modified functionality? My main questions so far are:

what do we gain from these changes?
what is broken with these changes?
do we need all the new functions (and in particular should they be exported)?
can't we achieve the same with the existing layout of Model and just implement condition/decondition by modifying the default values and missings?

src/model.jl

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

src/model.jl

phipsgabler · 2021-06-28T18:17:27Z

I am confused, isn't this wrong? Missing values are no observations but have to be sampled.

Right, that previously used a syntactic/static notion of "observation" (intersection of argument and variable names). I have fixed the printing now.

It's a bit difficult to parse all changes, it seems e.g. getargnames is just renamed but there is no new or modified functionality? My main questions so far are:

what do we gain from these changes?

I think that handling model arguments this way makes things much clearer. They are mostly bookkeeping, with some probabilistic meaning attached, and this should become clearer with moving away from all the focus on missing and defaults.

what is broken with these changes?

Only the existence of the defaults, currently, which affects the prob macro. This can be fixed temporarily by adding one more Argument subtype, I think (and I'll implement that to keep things non-broken for now). In the long term, the prob macros should be reimplemented or substituted by condition/decondition.

do we need all the new functions (and in particular should they be exported)?

Your're right, we don't need to export them. I have removed most of them.

can't we achieve the same with the existing layout of Model and just implement condition/decondition by modifying the default values and missings?

Somewhat. But IMHO this way things look much clearer, don't you think?

phipsgabler · 2021-07-01T14:51:17Z

Just to illustrate what this currently can do:

julia> @model function testmodel1(x; θ = 0.0)
                   s ~ InverseGamma(2, 3)
                   m ~ Normal(θ, sqrt(s))

                   x[1] ~ Normal(m, sqrt(s))
                   x[2] ~ Normal(m, sqrt(s))
               end
testmodel1 (generic function with 2 methods)

julia> m = testmodel1(randn(2))
Model testmodel1 given
    constants    θ
    observations x

julia> m = testmodel1([missing, 0.5])
Model testmodel1 given
    constants    θ
    observations x[2]

For now, that's just printing. I think these abilites can enable even nicer things in the future. And additionally, in my opinion the code is clearer.

phipsgabler · 2021-07-03T15:25:41Z

The prob macro now works, too! And given how smooth the change was, I believe this is a step in the right direction and towards condition/decondition.

Besides a lot of warnings, there is only one of the Turing integration tests failing, because somewhere in pointwise_loglikelihood the shape is not preserved:

loglikelihoods.jl: Test Failed at /home/philipp/.julia/dev/DynamicPPL/test/turing/loglikelihoods.jl:35
  Expression: logpdf(Normal(m, √s), xs[1]) == var_to_likelihoods["xs[1]"]
   Evaluated: -1.5235817173338913 == [-1.5235817173338913]
Stacktrace:
 [1] top-level scope at /home/philipp/.julia/dev/DynamicPPL/test/turing/loglikelihoods.jl:35
 [2] top-level scope at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.5/Test/src/Test.jl:1115
 [3] top-level scope at /home/philipp/.julia/dev/DynamicPPL/test/turing/loglikelihoods.jl:2

Is this a known thing?

CC @yebai

src/compiler.jl

src/model.jl

src/prob_macro.jl

phipsgabler · 2021-07-03T15:27:59Z

src/prob_macro.jl

        if argname in leftnames
-            push!(argvals, :(deepcopy(left.$argname)))
-            push!(missings, argname)
+            push!(argvals, :(Constant(deepcopy(left.$argname))))


I guess the old default arguments should be preserved, too:

Suggested change

push!(argvals, :(Constant(deepcopy(left.$argname))))

push!(argvals, :(Constant(deepcopy(left.$argname), getdefault(model, $argname)))

torfjelde · 2021-07-03T22:36:21Z

The essence of the implementation is that we can match model arguments and tildes at compile time:

But the current implementation also has this feature though.

I think that handling model arguments this way makes things much clearer. They are mostly bookkeeping, with some probabilistic meaning attached, and this should become clearer with moving away from all the focus on missing and defaults.

I'm a bit confused as to why we're distinguishing between Observation and Constant 😕 Why do we need Constant?

Also, making kwargs to be Constant is not the current behavior, right? E.g. if I make a kwarg missing, then it will be sampled, i.e. treated as missing observation.

what is broken with these changes?

Only the existence of the defaults, currently, which affects the prob macro. This can be fixed temporarily by adding one more Argument subtype, I think (and I'll implement that to keep things non-broken for now). In the long term, the prob macros should be reimplemented or substituted by condition/decondition.

I think also the behavior of kwargs as mentioend above.

can't we achieve the same with the existing layout of Model and just implement condition/decondition by modifying the default values and missings?

Somewhat. But IMHO this way things look much clearer, don't you think?

But if we're going to introduce condition and decondition, we also need the ability to do for variables rather than just arguments, right? For variables not part of the arguments, it seems like a ConditionContext is the best approach. But if we're already doing ConditionContext, then we might as well implement decondition and condition using a ConditionContext + ContextualModel?

We could even make isassumption a function that takes __model__, vn and left rather than expr (could even add __context__ too so that something like ConditionContext can change whether something is treated as observation or not), since it should still be possible to evaluate at compile-time. I've always been a bit puzzled about the current isassumption and why it needs to act on expr, so maybe I'm missing something.

github-actions · 2021-07-04T10:06:09Z

src/model.jl

+A `Model` struct with model evaluation function of type `F`, and arguments `arguments`.
 """
-    Model(name::Symbol, f, args::NamedTuple[, defaults::NamedTuple = ()])
+struct Model{F, argumentnames, Targs, internalnames, Tinternals} <: AbstractProbabilisticProgram


[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

struct Model{F, argumentnames, Targs, internalnames, Tinternals} <: AbstractProbabilisticProgram

struct Model{F,argumentnames,Targs,internalnames,Tinternals} <: AbstractProbabilisticProgram

github-actions · 2021-07-04T10:06:09Z

src/model.jl

+    model::Model{<:Any,argnames,Targs},
+    ::Type{T}


[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

model::Model{<:Any,argnames,Targs},

::Type{T}

model::Model{<:Any,argnames,Targs}, ::Type{T}

github-actions · 2021-07-04T10:06:10Z

src/prob_macro.jl

-                isdefined(ntr, arg) ||
-                isdefined(defaults, arg) && getfield(defaults, arg) !== missing
+        @assert all(getargumentnames(model)) do arg
+            isdefined(ntl, arg) || isdefined(ntr, arg) ||


[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

isdefined(ntl, arg) || isdefined(ntr, arg) ||

isdefined(ntl, arg) ||

isdefined(ntr, arg) ||

github-actions · 2021-07-04T10:06:10Z

src/prob_macro.jl

+                hasdefault(model, arg) &&
+                !ismissing(getdefault(model, arg))


[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

hasdefault(model, arg) &&

!ismissing(getdefault(model, arg))

hasdefault(model, arg) && !ismissing(getdefault(model, arg))

phipsgabler · 2021-07-07T15:39:44Z

The essence of the implementation is that we can match model arguments and tildes at compile time:

But the current implementation also has this feature though.

Yeah but this knowledge is hidden in generated code (by isassumption), and only treated negatively. For

@model function test(x, y)
	x ~ Normal(10, sqrt(y))
end
m = test(x, y)

there is no way the user can find out that y is "constant", and not on the LHS of a tilde, by inspecting m. The only way is looking at the output on a VarInfo (and with stochastic control flow, that can even be a nondeterministic observation!)

Now that I also have added internal variables, we have this:

julia> @model function gdemo(x, y, m0 = 0.0)
           s ~ InverseGamma(2, 3)
           m ~ Normal(m0, sqrt(s))
           x ~ filldist(Normal(m, sqrt(s)), length(y))
           for i in 1:length(y)
               y[i] ~ Normal(x[i], sqrt(s))
           end
       end
gdemo (generic function with 2 methods)

julia> model = gdemo(rand(3), rand(3), 10)
Model gdemo given
    constants:          m0
    observed variables: x, y
    latent variables:   s, m

julia> model = gdemo(rand(3), [0.5, missing, 0.5])
Model gdemo given
    constants:          m0
    observed variables: x, y[1], y[3]
    latent variables:   y[2], s, m

julia> model = gdemo(missing, [0.5, missing, 0.5])
Model gdemo given
    constants:          m0
    observed variables: y[1], y[3]
    latent variables:   x, y[2], s, m

I think that handling model arguments this way makes things much clearer. They are mostly bookkeeping, with some probabilistic meaning attached, and this should become clearer with moving away from all the focus on missing and defaults.

I'm a bit confused as to why we're distinguishing between Observation and Constant confused Why do we need Constant?

Constants are things that are not variables, i.e., not on the LHS of any tilde. They have no probabilistic meaning.

Also, making kwargs to be Constant is not the current behavior, right? E.g. if I make a kwarg missing, then it will be sampled, i.e. treated as missing observation.

They aren't all constants... obviously there might be bugs, but I do intend to match all of the old semantics:

julia> @model function test2(x; y = 1)
               y ~ Normal(10, sqrt(x))
       end
test2 (generic function with 1 method)

julia> test2(1)
Model test2 given
    constants:          x
    observed variables: y
    latent variables:   

julia> test2(1; y = missing)
Model test2 given
    constants:          x
    observed variables: 
    latent variables:   y

But if we're going to introduce condition and decondition, we also need the ability to do for variables rather than just arguments, right? For variables not part of the arguments, it seems like a ConditionContext is the best approach. But if we're already doing ConditionContext, then we might as well implement decondition and condition using a ConditionContext + ContextualModel?

This is something I and Hong are currently still investigating, and I am not really sure. You might want to take a look about how I updated the prob macro (OK, this actually buttresses your point, since the contexts there are still the same...). I need to think about this more.

We could even make isassumption a function that takes __model__, vn and left rather than expr (could even add __context__ too so that something like ConditionContext can change whether something is treated as observation or not), since it should still be possible to evaluate at compile-time. I've always been a bit puzzled about the current isassumption and why it needs to act on expr, so maybe I'm missing something.

I think I am essentially doing that in this implementation. The new isobservation takes a VarName, which has a symbol type parameter. Your idea for isassumption would require the same kind of approach to work at compile time.

github-actions · 2021-07-07T20:03:34Z

src/prob_macro.jl

+        Model(model.name,
+              model.evaluator,
+              $(to_namedtuple_expr(argnames, argvals)),
+              model.internal_variables)


[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

Model(model.name,

model.evaluator,

$(to_namedtuple_expr(argnames, argvals)),

model.internal_variables)

Model(

model.name,

model.evaluator,

$(to_namedtuple_expr(argnames, argvals)),

model.internal_variables,

)

github-actions · 2021-07-07T20:03:34Z

src/prob_macro.jl

+        model.name,
+        model.evaluator,
+        $(to_namedtuple_expr(argnames, argvals)),
+        model.internal_variables))


[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

model.internal_variables))

model.internal_variables,

))

torfjelde

Yeah but this knowledge is hidden in generated code (by isassumption), and only treated negatively. For
@model function test(x, y)
	x ~ Normal(10, sqrt(y))
end
m = test(x, y)
there is no way the user can find out that y is "constant", and not on the LHS of a tilde, by inspecting m. The only way is looking at the output on a VarInfo (and with stochastic control flow, that can even be a nondeterministic observation!)

Completely agree with this being a pro of this PR 👍

Constants are things that are not variables, i.e., not on the LHS of any tilde. They have no probabilistic meaning.

Yeah I follow that. My question was more towards "why do I need this separation?", in particular since, as you say, we potentially have stochastic control flow. But I guess in most cases it will make sense to just considered everything that could be an observation as an observation.

They aren't all constants... obviously there might be bugs, but I do intend to match all of the old semantics:

Okay, good 👍 I think I misunderstood:)

We could even make isassumption a function that takes __model__, vn and left rather than expr (could even add __context__ too so that something like ConditionContext can change whether something is treated as observation or not), since it should still be possible to evaluate at compile-time. I've always been a bit puzzled about the current isassumption and why it needs to act on expr, so maybe I'm missing something.

I think I am essentially doing that in this implementation. The new isobservation takes a VarName, which has a symbol type parameter. Your idea for isassumption would require the same kind of approach to work at compile time.

Yeah I realized this is one of those things that I just forget why we have it, and then when I start messing with it, I realized why 😅 See #271 where we end up back at a macro for the check (though also usable outside of the model, which can be nice).

Btw, I've left some comments in the code too, one which is of particular importance: how can the current approach work with @submodel? That is somewhat unclear to me atm 😕

torfjelde · 2021-07-03T22:02:11Z

src/model.jl

+[missing, 42]` -- then `x[1]` is not an observation, but `x[2]` is.)
+"""
+function isobservation(vn::VarName{s}, model::Model{<:Any,argnames}) where {s,argnames}
+    return (s in argnames) && isobservation(vn, getproperty(model.arguments, s))


Is the s in argnames needed if this approach is taken? E.g. why is it not okay for me to do something like Observation(x) ~ Normal() when x is not in the argnames?

E.g. could we do

return ( (s in argnames && isobservation(vn, getproperty(model.arguments, s))) || (s in internalnames && isobservation(vn, getproperty(model.internal_variables, s))) )

if we already have internal_variables available? This would allow:

Observation(x) ~ Normal() of some internal variable x to force it to be an observation. People have sometimes asked for something along those lines before, though it's already possible to do using @addlogprob!.

Seems like something like this is necessary to implement condition and decondition for latent variables, if we're not going the context-route?

Of course this still leaves the issue of "what do we do when left isn't defined yet, as in generate_tilde?". But then how do we then implement condition and decondition?

Observation(x) ~ Normal() seems like a really nice idea. You'd use that when you transforme passed-in data, right?

torfjelde · 2021-07-04T13:33:22Z

src/compiler.jl


    # extract observations from that
    modelinfo[:obsnames] = modelinfo[:allargs_syms] ∩ modelinfo[:varnames]
+    modelinfo[:latentnames] = setdiff(modelinfo[:varnames], modelinfo[:allargs_syms])


Btw, this will be incorrect for submodels and other similar non-top-level assume statements 😕

torfjelde · 2021-07-08T22:42:24Z

src/model.jl

+function Base.show(io::IO, ::MIME"text/plain", model::Model)
+    println(io, "Model ", model.name, " given")
+    print(io, "    constants:          ")
+    join(io, getconstants(model), ", ")
+    println(io)
+    print(io, "    observed variables: ")
+    join(io, getobservedvariables(model), ", ")
+    println(io)
+    print(io, "    latent variables:   ")
+    join(io, getlatentvariables(model), ", ")
+    return nothing
+end


This really needs some truncation, as the number of variables can be insanely large.

I would actually say that all the getobservedvariables, etc. should also be truncated or at least be an iterator, because there are models were you might be working with 10^5 variables, and thus mistakenly calling show on the model could be very slow (this is a huge pain with e.g. MCMCChains).

Sure. I was planning to replace it with some form of "compression" later, that, e.g., can collect x[1], ..., x[4], x[6], ..., x[10] into x[1:4], x[6:10].

...which only makes it better with "low-entropy missing patterns", though (like every other variable being missing). An iterator result and truncation seem like a good idea.

torfjelde

Cross-posting from Slack:

The issue is as follows:

@model inner(x) = x ~ Normal()
@model function outer(x)
    @submodel y = inner(x)
end

How do I specify that x within inner is the missing using that API without just making the input x of outer missing? This is effectively what we need a solution to if we want to support condition and decondition.

And even worse:

function @model inner(x)
    m ~ Normal(x, 1)
end
@model function outer(x)
    for i = 1:2
        @submodel $(Symbol("myprefix_$i)")) y = inner(x)
    end
end

How can I condition on myprefix_1.x? Doing this statically, i.e. trying to capture it from within the DPPL "compiler" as is done there, seems infeasible.

The last example also highlights the fact that the way the variable is referenced within the model isn't necessarily how it's referenced in the VarInfo, e.g. NamedDist.

torfjelde · 2021-07-09T00:19:49Z

src/model.jl

+isobservation(::VarName, ::Constant) = false
+isobservation(vn::VarName, obs::Variable{Missing}) = false
+isobservation(vn::VarName, obs::Variable) = !ismissing(_getindex(obs.value, vn.indexing))


Since this requires runtime to evaluate anyways, we might as well also fix some bugs, e.g. x[:, i] ~ MvNormal(...) currently doesn't work even though all the entries of X are missing:

isobservation(::VarName, value) = true isobservation(::VarName, ::Constant) = false isobservation(vn::VarName, obs::Variable{Missing}) = false isobservation(vn::VarName, obs::Variable) = isobservation(vn, _getindex(obs.value, vn.indexing)) isobservation(vn::VarName, value::Missing) = false function isobservation(vn::VarName, value::AbstractArray{>:Missing}) ismissings = (value .=== missing) if all(ismissings) # All are `missing` => assumptions. return false elseif any(ismissings) # Only SOME are `missing` => don't know how to handle this. error("$(vn) have some `missing` and some not; this is currently not supported") else # None are `missing` => observations. return true end end

Hm. But:

Isn't that intermingling two different things, the actual predicate and an error check in the model? Even if we can't handle mixed missings in some cases, a non-missing element of an array still is an observation, logically.

The check as you wrote it rules out too much, doesn't it? If the elements are accessed in a loop, and each tilda uses Normal, it should work. We can't detect the error case just from the observation being mixed, we have to take into account the distribution. That should be a separate check before or within the tilde calls.

torfjelde · 2021-07-09T00:25:49Z

src/model.jl

+    return :(NamedTuple{$filtered_argnames}(($(values...),)))
+end
+
+hasargument(model::Model, argname::Symbol) = isdefined(model.arguments, argname)


Would we want to do this at compile-time? E.g. an impl for Val or VarName?

Indeed, I think we do want to.

torfjelde · 2021-07-09T01:09:18Z

src/model.jl

+[missing, 42]` -- then `x[1]` is not an observation, but `x[2]` is.)
+"""
+function isobservation(vn::VarName{s}, model::Model{<:Any,argnames}) where {s,argnames}
+    return (s in argnames) && isobservation(vn, getproperty(model.arguments, s))


E.g. could we do

return ( (s in argnames && isobservation(vn, getproperty(model.arguments, s))) || (s in internalnames && isobservation(vn, getproperty(model.internal_variables, s))) )

if we already have internal_variables available? This would allow:

Observation(x) ~ Normal() of some internal variable x to force it to be an observation. People have sometimes asked for something along those lines before, though it's already possible to do using @addlogprob!.

Seems like something like this is necessary to implement condition and decondition for latent variables, if we're not going the context-route?

Of course this still leaves the issue of "what do we do when left isn't defined yet, as in generate_tilde?". But then how do we then implement condition and decondition?

phipsgabler · 2021-07-12T08:11:46Z

Btw, I've left some comments in the code too, one which is of particular importance: how can the current approach work with @submodel? That is somewhat unclear to me atm confused

Now there you raise a real problem... And I seriously don't know out of hand how to deal with it. Admittedly I completely ignored submodels, but they mess with my assumption that you can't add variable names at runtime.

I need to go thinking again.

Yeah I follow that. My question was more towards "why do I need this separation?", in particular since, as you say, we potentially have stochastic control flow. But I guess in most cases it will make sense to just considered everything that could be an observation as an observation.

True, that's a bit fuzzy. I believe "LHS of a tilde" is still a honest qualification to "potentially in the trace type", as the trace type for stoch. control flow is the union over all possible execution paths -- and we don't distinguish between the paths anyway. (To be exact, we only deal with "trace shapes", but the argument is the same.)

yebai · 2021-07-15T17:03:26Z

bors try

bors · 2021-07-15T21:39:43Z

try

Timed out.

phipsgabler · 2021-07-19T10:05:01Z

Will stall this for now; this approach has nice outputs for many models, but doesn't generalize to submodels and similar cases. Ideas of it will probably become part of a new implementation based on #279.

phipsgabler added 3 commits June 21, 2021 13:35

Different approach to how observations/missings are stored in the model

10a1505

Restructure and fix some things

781a86d

Adapt test cases

8244cdb

phipsgabler commented Jun 21, 2021

View reviewed changes

github-actions bot reviewed Jun 21, 2021

View reviewed changes

Remove fixed test group

66b228c

phipsgabler mentioned this pull request Jun 24, 2021

Refactor model for the AbstractPPL way #244

Closed

torfjelde reviewed Jun 24, 2021

View reviewed changes

src/model.jl Outdated Show resolved Hide resolved

Update src/model.jl

0a074bc

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

github-actions bot reviewed Jun 28, 2021

View reviewed changes

src/model.jl Outdated Show resolved Hide resolved

Fix some things and implement some suggestions

e79a0d0

github-actions bot reviewed Jun 28, 2021

View reviewed changes

phipsgabler commented Jun 28, 2021

View reviewed changes

src/model.jl Outdated Show resolved Hide resolved

phipsgabler added 4 commits July 3, 2021 13:32

Rename Observation -> Variable, simplify argument getters

852afb9

Move extraction functions out of show

ab8ce2c

Properly store default arguments

1979a64

Make prob macro code work

32184f2

github-actions bot reviewed Jul 3, 2021

View reviewed changes

phipsgabler commented Jul 3, 2021

View reviewed changes

phipsgabler added 2 commits July 4, 2021 11:47

Implement some JuliaFormatter suggestions

f070f82

Basic extraction of internal variables

1b32918

github-actions bot reviewed Jul 4, 2021

View reviewed changes

Implement proper separation of latent variables

9df5eb2

github-actions bot reviewed Jul 7, 2021

View reviewed changes

torfjelde mentioned this pull request Jul 8, 2021

Simplify isassumption check #271

Closed

torfjelde reviewed Jul 8, 2021

View reviewed changes

torfjelde reviewed Jul 10, 2021

View reviewed changes

yebai mentioned this pull request Jul 13, 2021

Benchmarking #248

Merged

Merge branch 'master' into phg/model-argument-refactoring

8c1a1f4

bors bot added a commit that referenced this pull request Jul 15, 2021

Try #268:

6de3d25

phipsgabler mentioned this pull request Jul 19, 2021

condition and decondition #278

Closed

phipsgabler closed this Jul 19, 2021

yebai deleted the phg/model-argument-refactoring branch January 28, 2022 20:29

	push!(argvals, :(Constant(deepcopy(left.$argname))))
	push!(argvals, :(Constant(deepcopy(left.$argname), getdefault(model, $argname)))

	struct Model{F, argumentnames, Targs, internalnames, Tinternals} <: AbstractProbabilisticProgram
	struct Model{F,argumentnames,Targs,internalnames,Tinternals} <: AbstractProbabilisticProgram

	model::Model{<:Any,argnames,Targs},
	::Type{T}
	model::Model{<:Any,argnames,Targs}, ::Type{T}

	isdefined(ntl, arg) \|\| isdefined(ntr, arg) \|\|
	isdefined(ntl, arg) \|\|
	isdefined(ntr, arg) \|\|

	hasdefault(model, arg) &&
	!ismissing(getdefault(model, arg))
	hasdefault(model, arg) && !ismissing(getdefault(model, arg))

Different approach to how observations/missings are stored in the model #268

Different approach to how observations/missings are stored in the model #268

Uh oh!

Conversation

phipsgabler commented Jun 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

devmotion commented Jun 21, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

phipsgabler commented Jun 28, 2021

Uh oh!

phipsgabler commented Jul 1, 2021

Uh oh!

phipsgabler commented Jul 3, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

torfjelde commented Jul 3, 2021

Uh oh!

github-actions bot Jul 4, 2021

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jul 4, 2021

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jul 4, 2021

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jul 4, 2021

Choose a reason for hiding this comment

Uh oh!

phipsgabler commented Jul 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot Jul 7, 2021

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jul 7, 2021

Choose a reason for hiding this comment

Uh oh!

torfjelde left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phipsgabler commented Jun 21, 2021 •

edited

Loading

phipsgabler commented Jul 7, 2021 •

edited

Loading

torfjelde Jul 8, 2021 •

edited

Loading

phipsgabler commented Jul 12, 2021 •

edited

Loading