change defaults: @addlogprob! not included in PriorContext()

[thorek1]

Would it be possible to change the default behavior for @addlogprob! and not have it included in the PriorContext()

I understand the current solution is to do this:

if DynamicPPL.leafcontext(__context__) !== Turing.PriorContext()
    Turing.@addlogprob! myloglikelihood(x, μ)
end

as in the default is that it is included.

In my use case (see this issue) it would make the syntax much more user-friendly for end-users

[devmotion]

IMO (at first glance at least) the current default is reasonable since @addlogprob! just indicates that something is added to the joint log density that is computed by default.

That being said I think the syntax could be improved: #390

[thorek1]

what about setting up another macro (e.g. @addlogprob_noprior) that does this?

the request is triggered by the fact that some backends care about the context. I agree, from a user perspective that this should not be relevant. hence, my desire to have a simple one-line solution

[devmotion]

I think #390 would be cleaner than a separate macro - in Turing, every tilde statement is either an assumption ("prior") or an observation ("likelihood"), so stating explicitly to which category the custom log density calculation belongs would rule out any confusion (and could ideally be treated correctly then also when someone implements eg a custom prior context, which @addlogprob_noprior etc. would not know about).

[torfjelde]

I'm very happy with adopting the syntax in #390 tbh. Shouldn't be too difficult to implement either?

[thorek1]

#390 is suggesting to have an option after the macro call, right? if so, sounds good to me

[penelopeysm]

#885 will solve this (once merged to main) as prior and likelihood are cleanly separated.

[thorek1]

What's the new syntax looking like? I guess I will find it in the docs?!

[penelopeysm]

It's @addloglikelihood (; logprior=0.0, loglikelihood=mylogprob()). Besides, just @addlogprob mylogprob() will add to the log-likelihood, so in fact it does directly implement your original suggestion.

0.37 of DynamicPPL will be quite a big change as PriorContext will also be removed, so it's likely that anything that was using it will need to adapt. I've seen the Pigeons.jl code a few times and I think there will be quite a few changes needed there (although I'm more than willing to help, as I've told the Pigeons devs before :))

It'll first be documented in DynamicPPL's docs: https://turinglang.org/DynamicPPL.jl/ but then it will take time for us to make Turing compatible, and once that's done we can update the main docs https://turinglang.org.

((I argued for @addloglikelihood and @addlogprior, but this was apparently deemed unnecessary)