Model-dispatching and Experimental submodule

[torfjelde]

This PR introduces a DynamicPPL.Experimental which is intended to be a submodule where we can put more, well, experimental features, e.g. SimpleVarInfo from #309. It also introduces the evaluatortype method, as discussed multiple times before, which allows one to dispatch on model-definition using the following pattern:

@model function demo()
    ...
end

f(::Model{evaluatortype(demo)}) = ...

I've been using this extensively in a bunch of projects over the past month to do neat things like:

• Associate specific converter-methods with models to convert "groups" of variables in a MCMCChains.Chains to their correct shape, if the chain came from the model itself.
• Associating manual implementation of logjoints with a model for maximum performance during sampling while maintaining the convenience of the @model definition for analysis of the chain, e.g. using predict, etc. + testing correctness of manual implementation.
• Defining data_to_model_args methods for different models to disentangle the data processing from the model definitions, and then just provide a simple "gluing" function to convert the data to arguments compatible with a specific model.

In the future this could also allow users to tell samplers about properties of their model which can then be exploited to improve performance, e.g. isdynamic to tell the sampler to use UntypedVarInfo, and much more.

This evaluatortype is somewhat "hacky", but as we've discussed before, there doesn't seem to be any other way to things atm. Hence this is introduced in the Experimental submodule.

[torfjelde]

Tests are passing locally btw 👍

[devmotion]

IIRC we had a discussion on Slack with some alternative approach that avoids return_types which can't be relied on generally?

[torfjelde]

Uhmm I thought we concluded the opposite, that indeed the only way we could do this was through return_types 😅 @phipsgabler ?

I'll have a look; hopefuly it's still there (stupid Slack!).

[torfjelde]

So I just looked and IIUC the other viable approach was just explicit instantiation, which doesn't seem like the way to go 😕

[devmotion]

Why is that not a viable approach? This works always and seems to be the natural approach:

f(x) = false

@model demo() = x ~ Normal()
f(::Model{typeof(demo())}) = true

We could provide an helper function that avoids having to type Model{typeof(...)} but I'm not completely sure if it would be much shorter (and probably it would be less obvious). And as with evaluatortype one could also dispatch on other type parameters of Model.

[torfjelde]

But this requires explicit instantiation of demo though? Sure it works nicely when there are no arguments, but what about a model such as:

@model demo(x::Matrix, y:Vector)

?

Of course it's still possible to just do demo(randn(2, 2), [1.0]) or whatever, but that is def not more intuitive nor easier from a user-perspective.

[torfjelde]

Btw, could you maybe outline the exact reason why the usage of return_types is a bad idea? I fear I might be missing something. AFAIK it's because sometimes it can return Any (I can't see that every happening in modeltype though), which I think would then be correctly handled by erroring. Is there something else?

[devmotion]

Won't this lead to issue with serialization though? Hmm, maybe I'm not understanding properly.

I don't think it would cause any problems if it is used as intended. It should be used as

module M

using DynamicPPL

@modelevaluator f(....)

end

....

mytrait(x) = false
mytrait(::typeof(f)) = true

end

or

module M

using DynamicPPL

struct F{A}
    a::A
end

@modelevaluator (f::F)(....)

end

....

mytrait(x) = false
mytrait(::F) = true

end

If you load the module it should be fine to serialize and deserialize. The main difference here is that we do not define types or instances for users automatically which leads to problems depending on scope etc. but here users that want to define a model and accompanying traits without instantiating it have full control over defining it in the correct scope, with their types or as a regular function, and can avoid all these const issues.

I'm not opposed to having such a macro (this is effectively what I did at first for @SubModel, remember?), but IMO this seems far from as use-friendly as modeldef(demo)

I think there are two main reasons why we should not include modeldef based on types and why basically nobody should use it:

• There are no guarantees that it returns the desired type
• It seems conceptually wrong to try to extract the type from the model function if you can get it automatically when you define the evaluator separately

Btw, you're aware that we have this piece of code in DPPL sweat_smile

Yes, and I don't like it 😄

[torfjelde]

There are no guarantees that it returns the desired type

I'm a bit confused by this. Is this an issue with return_types itself or are you talking about cases where you have multiple definitions of the same model and so you could end up with the wrong one if you don't specify the arguments?

It seems conceptually wrong to try to extract the type from the model function if you can get it automatically when you define the evaluator separately

When you write it like that, sure I agree:) From the user's perspective it's instead "Man, it sure doesn't seem intuitive that I have to define the evaluator separately rather than just extracting it from the model definition" 😅 But, even so, this means you have to define the evaluator separately which to me seems like it makes it easier to make a mistake from the user-side.

Yes, and I don't like it

Neither do I:) But it seems like it's "worth the cost".

[devmotion]

Btw, could you maybe outline the exact reason why the usage of return_types is a bad idea? I fear I might be missing something. AFAIK it's because sometimes it can return Any (I can't see that every happening in modeltype though), which I think would then be correctly handled by erroring. Is there something else?

The main problem I am aware of is in fact that is not guaranteed to return typeof(f(args...)) but it can be any supertype (so at least theoretically it could also be an abstract subtype of Model and you might redefine some other traits for some other models). Moreover, it does not work for builtin functions (JuliaLang/julia#39299). I also assume it can return inference results in the wrong world age if you use redefinitions and a weird setup.

IMO already the first problem is reason enough to not use it here since there seems to be a conceptually better alternative that does not involve return_types.

[devmotion]

When you write it like that, sure I agree:) From the user's perspective it's instead "Man, it sure doesn't seem intuitive that I have to define the evaluator separately rather than just extracting it from the model definition" sweat_smile But, even so, this means you have to define the evaluator separately which to me seems like it makes it easier to make a mistake from the user-side.

I wonder if they could be defined together but I assume then the syntax would become a bit confusing. The evaluator has to be defined in the same way as the @model since there actually the rewriting of the tilde expressions etc happens. The model would only require the evaluator and the default arguments.

A probably quite bad idea would be make

@model function mymodel(x, y; kwargs...)
    ...
end

define two definitions of mymodel: mymodel(x, y; kwargs...) and mymodel(::AbstractContext, ....), i.e., the evaluator., where the former would just construct a model with the latter. This would mean, however, that @model demo() = ...; @model demo() = ... would redefine the evaluator and possibly this also leads to serialization issues.

[devmotion]

I think this name is confusing since it is not correct. The type of the model is Model{F,...} including all the other type parameters as well. This just returns an abstract supertype.

[torfjelde]

Agree with that, but:

1. Haven't come up with a better name yet (very open to suggestions).
2. Intuitvely I'd expect modeltype(model) !== typeof(model) since otherwise it's useless, and so I'd likely check the docstring for what it was doing before using it if I wasn't aware.

[torfjelde]

closed in favor of #316