Simplify the tilde-pipeline in DynamicPPL

[mhauru]

TODO

Edit this issue with more details about what needs to be done

Related issues

• https://github.com/TuringLang/DynamicPPL.jl/issues/722
• https://github.com/TuringLang/DynamicPPL.jl/issues/720
• https://github.com/TuringLang/DynamicPPL.jl/issues/402

[yebai]

As briefly discussed with @mhauru, we could introduce a new (internal) macro DynamicPPL.@get_paramter_type(x::VarName) to replace the current hard-coded logic for determining whether a parameter is missing, observation, fixed or random variable. This is related to @isobservation in https://github.com/TuringLang/DynamicPPL.jl/issues/714 (I don't think we need @observed_value(x) since this is available in the model scope).

[torfjelde]

don't think we need @observed_value(x) since this is available in the model scope

Maybe I misunderstand, but this is not available in the model scope in the case where x is conditioned using condition, right? That's the problem the @value in the issue is meant to address. Ofc, technically you could get it from the __context__, but you need to call the right function on this context, etc. which seems overly complicated for the end-user, no?

[torfjelde]

(internal) macro DynamicPPL.@get_paramter_type(x::VarName) to replace the current hard-coded logic for determining whether a parameter is missing, observation, fixed or random variable.

So, currently, the generation of these branches is done by separate functions, e.g. generate_tilde_assume or something in DynamicPPL.jl, right? Do you want to replace these functions that act on Expr with macros that are embedded in the model? If so, isn't that just doing the same thing?

I'm probably not understanding correctly here :thinking:

[yebai]

his is not available in the model scope in the case where x is conditioned using condition, right? That's the problem the @value in the issue is meant to address. Ofc, technically you could get it from the context, but you need to call the right function on this context, etc. which seems overly complicated for the end-user, no?

Not sure what is the exact behaviour of left ~ distr if left is a conditioned variable. If it is not yet exposed to the model scope, we should modify the tilde pipeline to ensure that conditioned variables are exposed, e.g.,

# final step of tilde pipeline
left = conditioned_value

This would ensure that conditioned variables are accessible in the model scope (s.t. local scope constraints).

So, currently, the generation of these branches is done by separate functions, e.g. generate_tilde_assume or something in DynamicPPL.jl, right? Do you want to replace these functions that act on Expr with macros that are embedded in the model? If so, isn't that just doing the same thing?

This is not about adding new functionality. The purpose is to refactor the code and consolidate it into a single macro/function that we can use internally but exported as a public API.

[yebai]

It was proposed to have a single unified tilde instead of various tilde_* functions https://github.com/TuringLang/DynamicPPL.jl/issues/910#issuecomment-2858556681

[penelopeysm]

In the next release of DPPL, there are only two functions: tilde_assume!! and tilde_observe!!. Is this pretty much complete? It is possible to unify both into a single tilde although I think the resulting function will end up having a structure like this, so I think it's easier to keep it separate for now.

function tilde!!(is_observed::Bool, ...)
    if is_observed
        return tilde_observe!!(...)
    else
        return tilde_assume!!(...)
    end
end

Even the phrase 'tilde-pipeline' is no longer really accurate, since it is only a depth of 1 now.

I think in the future if we have different types for conditioned variables, etc. then it may make sense to instead have

# these are observe
tilde!!(ctx, v::ConditionedVariable, right, vn, vi) = ...
tilde!!(ctx, v::FixedVariable, right, vn, vi) = ...
tilde!!(ctx, v::Literal, right, vn, vi) = ...
# this is assume
tilde!!(ctx, v::MissingVariable, right, vn, vi) = ...

But that's a while away, and I think larger-scale design changes should be part of a different milestone.

[mhauru]

I would be happy to consider this done, and open a separate issue if we feel like joining up the two tilde functions makes sense. My instinct is that it would only make sense after we've refactored some of the logic around how conditioning works and maybe unified the treatment of variables given as arguments and those set using condition, if at all.