Skip to content

Mark ALL functions that take DynamicPPL.Model as an argument, anywhere, as produceable? #217

@penelopeysm

Description

@penelopeysm

Inspired by TuringLang/Turing.jl#2778, I'm currently thinking about whether it would straight-up make sense to declare ALL functions that take DynamicPPL.Model (and maybe DynamicPPL.AbstractVarInfo) as an argument with might_produce.

The rationale is similar to that described in that PR: of course there are many functions that take Models and do not produce, so this seems like an overly broad generalisation — but none of them are likely to occur within the body of a model. That means that false positives are extremely unlikely to occur.

Conversely, we do want to make sure that every model evaluator function — plus any variations on it, if the model has keyword arguments (see #197) — is marked as produceable. The effect of not doing so, i.e. false negatives, is that the user is silently given wrong results, which is disastrous!!

This is specifically with the purpose of supporting:

  1. models with keyword arguments;
  2. submodels;
  3. (and consequently, submodels with keyword arguments).

Right now, (1) and (3) are handled by making the user declare the function as produceable themselves, and (2) is handled by TuringLang/Turing.jl#2778.

However, we could fix all of this in one fell swoop with the above strategy. I think the downside is very small.

Note that we would need new functionality in Libtask to do this; we can't currently do this at the Turing level. The reason is because might_produce takes a specific method signature and we want to be more general than that: we want to mark all method signatures that have Model anywhere in their signatures as being produceable.

Essentially, we want to modify this bit:

function stmt_might_produce(x, ret_type::Type)::Bool
# Statement will terminate in an unusual fashion, so don't bother recursing.
# This isn't _strictly_ correct (there could be a `produce` statement before the
# `throw` call is hit), but this seems unlikely to happen in practice. If it does, the
# user should get a sensible error message anyway.
ret_type == Union{} && return false
# Statement will terminate in the usual fashion, so _do_ bother recusing.
is_produce_stmt(x) && return true
Meta.isexpr(x, :invoke) && return might_produce(get_mi(x.args[1]).specTypes)
if Meta.isexpr(x, :call)
# This is a hack -- it's perfectly possible for `DataType` calls to produce in general.
f = get_function(x.args[1])
_might_produce = !isa(f, Union{Core.IntrinsicFunction,Core.Builtin,DataType})
return _might_produce
end
return false
end

For obvious reasons, we can't introduce a hard dep on DynamicPPL. I'm thus currently a bit unsure about the best interface for this; but I am pretty sure that in principle the idea is sound.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions