joshspeagle / dynesty

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

sample_* don't respect rstate #291

Closed segasai closed 3 years ago

segasai commented 3 years ago

While looking at the sampling code I noticed that the sample_rslice and other methods do not use rstate at all. They just set rstate to np.random

https://github.com/joshspeagle/dynesty/blob/6843ca1e0df26410679cf39cb2ce43695782698f/py/dynesty/sampling.py#L150

This clearly is incorrect both for single and mult-threaded cases. The fix though is somewhat not trivial as one needs to somehow either pass random state or a seed generated from a parent random state. But one can't just send the same random state. Also generating a random state is somewhat heavy ~ 0.1, so it'd be better to avoid recreating them all the time.

segasai commented 3 years ago

It looks like this https://numpy.org/doc/stable/reference/random/bit_generators/pcg64.html is the right way to deal with this.

joshspeagle commented 3 years ago

I agree this is a pretty serious bug you've uncovered; I'm not sure where this originated.

Thanks for implementing a fix in #292 so quickly. Since I've merged that in I'll close this for now.

segasai commented 3 years ago

Thanks for merging. It looks like the bug was there from the very beginning. It's just because numpy random generator had a global state, it was not detectable when using np.random.seed(X)