Add diagnostic for model logp

[junpenglao]

In PyMC3 we did not recorde the model logp to the trace. Today I came across this explanation from Betancourt, which I think it is quite excellent and we should consider adding it to the trace or output the diagnoistics: http://discourse.mc-stan.org/t/convergence-failure-maybe-in-brms/4148/7. Copied below:

lp__ tends to be extremely sensitive to the autocorrelation of the Markov chain and hence provides a reasonable bound on how well any function of the variables will converge. In other words, ensuring that the diagnostics for lp__ are good gives you the strongest evidence that your fit is okay but if you focus only on a few variables and carefully check the diagnostics for those variables then you may be able to ignore lp__ for that very specific context.

[fonnesbeck]

This can be calculated from the trace without having to track it, right?

[twiecki]

Sounds like a good idea to add it to the trace.

[junpenglao]

This can be calculated from the trace without having to track it, right?

Yes. We can add it at the end of sampling using trace.add_value.

[MBrouns]

Is someone working on this or can I pick this up?

[springcoil]

I don't think anyone is working on it.

…

On Thu, May 17, 2018 at 7:01 PM, MBrouns ***@***.***> wrote: Is someone working on this or can I pick this up? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2971 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA8DiEaiNVz9IvfcXxctTEHQbr47keLTks5tzbsNgaJpZM4T6XJ0> .

-- Peadar Coyle Skype: springcoilarch www.twitter.com/springcoil peadarcoyle.wordpress.com

[rlouf]

@MBrouns are you on this? Otherwise I'd be happy to pick it up.

[MBrouns]

I made a start but got stuck somewhere halfway. Go ahead!

[rlouf]

I'll try my best 😄 It could be helpful if you told me where you got stuck? I quickly looked at the code base and it seems that the Factor class has a lot of

[junpenglao]

FYI, I think there are two ways to do it, either add the logp (name it logp__) as deterministic, which we are already doing for SMC:
https://github.com/pymc-devs/pymc3/blob/12694ddf0daebffea1570ede0b60a25b0aba30a7/pymc3/step_methods/smc.py#L117-L123

Or we compute it after sampling using [tmodel.logp(point) for point in trace] (need to be careful to make sure all chains are evaluated) and add it to the trace using trace.add_values.

The main challenge would be to benchmark it to see which one is faster

[twiecki]

If it doesn't slow things down (which I can't imagine it would) I think the more elegant method would be to add it to the trace while sampling.

[junpenglao]

It would also simplify the SMC code so I fully support this approach.

[eigenfoo]

Hi! Completely new to the PyMC3 underbelly, so just for my understanding:

The idea here is to add something like

with model:
    llk = pm.Deterministic('logp__', model.logpt)

to the __init__ function of the BaseTrace class (in backends/base.py), right? And then we would expose the logp in some nice way in the MultiTrace class? (And of course, document this etc.)

This would mean we could remove all this model.logpt business out of the step_methods modules, to avoid repeating the code in every sampler.

Is that correct? If it is, and if @MBrouns and @rlouf don't object, I'd be happy to submit a PR.