Skip to content

Conversation

@torfjelde
Copy link
Member

@torfjelde torfjelde commented Jul 12, 2021

We've received multiple requests to allow observing on something that isn't part of the arguments, e.g.

@model function demo(args)
    x = args.x
    x ~ Normal()
end

We cannot currently support exactly this, but now that we have proper macro-handling with @model we can easily support something like

@model function demo(args)
    x = args.x
    @observe x ~ Normal()
end

This PR introduces such an @observe.

It does so by passing keyword arguments through all the generate_mainbody! calls eventually making it's way to the generate_tilde methods, which is really what we want to alter. I do this because it allows us to re-use the generate_mainbody!, thus ensuring that we generate code that @model would have.

I'm generally not a big fan of the "pass kwargs around like there's no tomorrow!!!"-approach, hence I'm happy to hear suggestions of how we can do this in a better way. I think it would be nice to be able to do this in a bit more extensible manner so that we can write macros that only alter the ~ without having to copy-paste the entire definition of generate_mainbody.

@yebai
Copy link
Member

yebai commented Jul 13, 2021

@phipsgabler and I have talked about more flexible conditioning via providing a model API, i.e. conditioning(m, vars). See AbstractPPL. Regarding the @observe macro, I am slightly against introducing it; it doesn't really solve the underlying problem, i.e. secondary variables derived from arguments are not treated as observed variables. It should be possible to fix this from the compiler level, I think.

@torfjelde
Copy link
Member Author

I 100% agree that this PR doesn't provide a complete fix and there definitively should be a better approach to doing this, but I think this is unfortunately a bit further off into the future. Hence the @observe macro will work as hotfix while we wait for a more complete way of handling this. Once we have this, we simply deprecate @observe and eventually remove it:)

It's similar to @submodel: ideally we shouldn't need a macro to do this, but doing it without a macro isn't feasible right now. So in the mean time we have a macro for doing this.

@yebai
Copy link
Member

yebai commented Jul 13, 2021

Overall, we'd like to stick to the principle of minimising the use of macros in modelling syntax. If we want to temporarily support this, maybe replacing @observe x ~ Normal() with x ⩪ Normal() ( stands for dotsim) to force variable x to be an observation?

@torfjelde
Copy link
Member Author

Overall, we'd like to stick to the principle of minimising the use of macros in modelling syntax. If we want to temporarily support this, maybe replacing @observe x ~ Normal() with x ⩪ Normal() ( stands for dotsim) to force variable x to be an observation?

Hmm, not too big of a fan as it introduces new syntax. The idea of the @observe is to have an answer to "hey can I observe on something that isn't in the arguments?" and we can go "if you really want to, you can use @observe" rather than it being something we actively encourage (since as you said, we want to handle this in a better way in the future).

Agree with minimizing usage of macros, but I think they're useful for these "not officially encouraged usage but if you really want this feature right now, here it is", e.g. @submodel, @observe, @reparam.

@yebai
Copy link
Member

yebai commented Jul 17, 2021

bors try

bors bot added a commit that referenced this pull request Jul 17, 2021
@torfjelde
Copy link
Member Author

Pretty certain we need to also upper-bound Distributions in DPPL tests until we've updated the tests.

@bors
Copy link
Contributor

bors bot commented Jul 17, 2021

try

Build failed:

return sym
end
function generate_mainbody!(mod, found, expr::Expr, warn)
function generate_mainbody!(mod, found, expr::Expr, warn; tilde_kwargs...)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the new kwargs always tilde_kwargs? If yes, I'd suggest consistently using the latter name, also in the above methods.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is right now yes

@phipsgabler
Copy link
Member

phipsgabler commented Jul 19, 2021

I'm generally not a big fan of the "pass kwargs around like there's no tomorrow!!!"-approach, hence I'm happy to hear suggestions of how we can do this in a better way. I think it would be nice to be able to do this in a bit more extensible manner so that we can write macros that only alter the ~ without having to copy-paste the entire definition of generate_mainbody.

I can think of multiple ways to rewrite the compiler code so that it looks a bit like Cassette overdubbed code -- very compositional, and ever place we call a generate_ function being replaceable by dispatch on "contexts". But I guess that's kind of overkill for now.

Also, these contexts would need to be updated and threaded through, to allow information to propagate.

On the other hand, if we do have such a modularized system, it'd be much easier to write abstract interpreters and other fancy stuff with it.


And I also prefer @observe x ~ Normal() over x ⩪ Normal(). Using macros is, even if ad hoc, at least a consistent pattern of "extensions".

@torfjelde torfjelde mentioned this pull request Jul 19, 2021
3 tasks
@torfjelde
Copy link
Member Author

bors try

bors bot added a commit that referenced this pull request Jul 22, 2021
@bors
Copy link
Contributor

bors bot commented Jul 22, 2021

try

Build failed:

@yebai
Copy link
Member

yebai commented Mar 17, 2022

Closed in favour of TuringLang/Turing.jl#1736 (comment)

@yebai yebai closed this Mar 17, 2022
@yebai yebai deleted the tor/force-observe branch March 17, 2022 17:57
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

Successfully merging this pull request may close these issues.

4 participants