search

TuringLang / MCMCDebugging.jl

MCMCDebugging.jl: debugging utilities for MCMC samplers
MIT License
4 stars 5 forks source link

Why sampling from the joint is called `marginal`? #3

Open Red-Portal opened 5 months ago

Red-Portal commented 5 months ago

Hi,

I am currently looking at the code to merge MCMCTesting.jl. But I found the naming of rand_marginal a little unusual since it seems to sample from the joint $p(x, \theta)$ but not from the marginals of $x$ or $\theta$. Is there a reason for this naming? And would it be possible to rectify this?

xukai92 commented 5 months ago

you are right. better to call it rand_joint or something similar

Red-Portal commented 5 months ago

@xukai92 Are any packages within the Turing ecosystem currently dependent on MCMCDebugging.jl at the moment? How critical are breaking changes?

xukai92 commented 5 months ago

nothing critical. it was only used when developing the coupled HMC methods and Riemannian HMC during the research phase

Red-Portal commented 5 months ago

Okay. Then, I'll take my creative liberties. However, I feel that using some of these within the CI pipeline for Turing's algorithms would be useful to spot bugs like this in the future. So maybe it would be worth actually using this.

xukai92 commented 5 months ago

that's a good point. i remember we used to have it (Geweke test) somewhere either in Turing (e.g. https://github.com/TuringLang/Turing.jl/blob/56f64ec5909cec4a5ded4e28555c2b289020bbe1/test/skipped/hmcda_geweke.jl#L4) and it has been useful to capture bugs. but it was disabled as the primitive implementation was a bit hard to maintain. i believe this issue will go away with MCMCDebugging + MCMCTesting consolidated to a well-maintained package and hopefully all inference package could include Geweke tests

Red-Portal commented 5 months ago

I am pretty optimistic. I have been using the newer tests in MCMCTesting in my own works' CI workflow, and they have been surprisingly useful. I was having a very occasional test fail on an RJMCMC code, which I thought was just Type-I error, but turned out to be a veeery subtle reversibility bug (took 2 months to realize that is was indeed a bug)