search

joshspeagle / dynesty

Dynamic Nested Sampling package for computing Bayesian posteriors and evidences
https://dynesty.readthedocs.io/
MIT License
347 stars 76 forks source link

n_cluster breaks slice sampling #270

Closed segasai closed 3 years ago

segasai commented 3 years ago

I never quite understood what n_cluster does (and it is not tested by the test suite), but as far as I can see it completely invalidates the slice sampling. As there is never a guarantee that if you randomly sample in the dimensions other than n_cluster https://github.com/joshspeagle/dynesty/blob/9f296cf96624c8a6db87d8096bae204bcb999682/py/dynesty/sampling.py#L705 , you'll end up in the logl>loglstar region, thus invalidating the sampling.

TBH I'm inclined to disable n_cluster alltogether as atm it's completely untested (and likely not working)

joshspeagle commented 3 years ago

I was confused about this since I did not remember introducing anything related to n_cluster at first. Looking back, it seems like that feature was introduced in this PR to differentiate between parameters that were being bounded/clustered and those that were being ignored. It can (probably?) just be rolled back or changed.

segasai commented 3 years ago

I think that PR was incorrect for slice sampling at least. I am not certain it is correct for other sampling methods either. I can certainly understand restricting ellipsoidisation to only some dimensions, but I don't think the sample_* methods should have been touched at all. I'm inclined to disable this unless this is cleaned up.

joshspeagle commented 3 years ago

I'm totally fine with that.

segasai commented 3 years ago

I disabled ncdim!=ndim for slice sampling in #271 and removed it from slice sampling code. I still kept it for other samplers.