fix bug in elliptical slice sampling

STOR-i / GaussianProcesses.jl

A Julia package for Gaussian Processes

https://stor-i.github.io/GaussianProcesses.jl/latest/

Other

308 stars 53 forks source link

fix bug in elliptical slice sampling #180

Closed Red-Portal closed 3 years ago

Red-Portal commented 3 years ago

Hi, found a bug in elliptical slice sampling. Elliptical slice sampling's transition probability uses the likelihood, not the joint probability. It should now sample from the correct posterior. The previous version should have resulted in the prior to be stronger than expected.

coveralls commented 3 years ago

Pull Request Test Coverage Report for Build 665

5 of 5 (100.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 76.622%

Totals
Change from base Build 635:	0.0%
Covered Lines:	1488
Relevant Lines:	1942

💛 - Coveralls

maximerischard commented 3 years ago

I see, this makes sense. Would there be a way to test the correctness of the sampler? For example by checking the prior can be recovered from data simulated from the prior?

Red-Portal commented 3 years ago

Hi, @maximerischard I actually have thought about that issue in a personal project, but that would be an identical issue to hypothesis testing, where type-I and type-II need to be traded off for each other. In fact, the bug that I'm trying to fix with this patch isn't identifiable by something like a prior predictive check. So I doubt that there is a practical way to do that.