Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding a variable to missings in a Model does not propagate to a submodel #273

Open
torfjelde opened this issue Jul 9, 2021 · 4 comments

Comments

@torfjelde
Copy link
Member

julia> @model demo_inner(x) = x ~ Normal()
demo_inner (generic function with 1 method)

julia> @model demo(x) = @submodel demo_inner(x)
demo (generic function with 1 method)

julia> m = demo(1.0);

julia> m()
1.0

julia> m_missing = Model{(:x, )}(m.name, m.f, m.args, m.defaults);

julia> m_missing()
1.0

And the issue is that I don't even know what the correct solution is here! E.g. you can for example have x in both parent- and submodel, and so setting x as missing as in the above example is ambiguous even if we could propagate the information.

Only proper solution I see to this is a context which allows forcing missing. This could then accomodate for variables of submodels which are potentially prefixed.

@devmotion
Copy link
Member

It seems the main issue is that currently it is not clearly defined if missings is based on the name of the arguments or name of random variables in the model. And they differ in submodels.

I always thought missings should be based on the random variables (and probably also allow us to specify that e.g. only x[2] is missing). Thus in the example one would have to include demo_inner.x in missings but not x. This would fix the confusion and surprises, it seems, and also allow to propagate missings by subsetting missings of the parent model in the @submodel macro.

@torfjelde
Copy link
Member Author

Agreed, but:

  1. Maybe having missings in the type of the model isn't the best approach then, given that stuff like x[1] won't always be inferrable at compile-time, e.g. x[i].
  2. This requires making isassumption take into account the "namespace"/prefix being used, i.e. it needs to look at the varinfo.

Maybe it would make more sense to do this together with a ConditionContext? E.g. allow "deconditioning" by making isassumption return true. It's one of the things I mention as possible usecases in #274.

@devmotion
Copy link
Member

I don't think 2 is needed. I assume one can call _evaluate not with the model but with merge_missings(model, __model__) where merge_missings adds the missings of __model__ that begin with Symbol(model.name, ".") to model?

@torfjelde
Copy link
Member Author

I don't think 2 is needed. I assume one can call _evaluate not with the model but with merge_missings(model, model) where merge_missings adds the missings of model that begin with Symbol(model.name, ".") to model?

But model.name isn't sufficient to guarantee uniqueness, so you can have other completely different prefixes for the variable. Maybe I misunderstand what you're saying? 😕

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants