joshspeagle / dynesty

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

fix the bug in sample_batch when not enough live points exist #236

Closed segasai closed 3 years ago

segasai commented 3 years ago

Hi I was struggling with a problem when I use rslice sampler

dsampler = dynesty.DynamicNestedSampler(
        wrap_like,        prior_transf,
        periodic=[2, 4],
        ndim=11,        pool=pool,
        queue_size=24,        sample='rslice',
        use_pool={
            'loglikelihood': True,
            'evolve': False
        })

    dsampler.run_nested(dlogz_init=1,
                        nlive_init=500,
                        nlive_batch=500,
                        wt_kwargs={'pfrac': .9},
                        stop_kwargs={'pfrac': .9})

and I was getting all the time

RuntimeError: Slice sampling appears to be stuck!

errors.

The investigation showed that the problem is the following: when sample_batch is called https://github.com/joshspeagle/dynesty/blob/eb72a2e02720cf7b4384728b995705e1d303cc38/py/dynesty/dynamicsampler.py#L943

it can have the logl interval that only contains a handful of (top) points from the base run. When the rewinding of the chain happens here: https://github.com/joshspeagle/dynesty/blob/eb72a2e02720cf7b4384728b995705e1d303cc38/py/dynesty/dynamicsampler.py#L1101 only part of the top nblive points satisfy the >logl_min criteria. Therefore when _new_point happen below, the slice sampling doesn't work because because there is a very large chance that for a random direction the slice won't hit the >logl_min area.

The patch here fixes that. Also in fact there was a bug previously, that when preparing a list of live points to sample from, one point was always below logl_min.

Finally also there were two variables loglmin and logl_min in the same scope (I initially thought it was the source of my problem). I renamed one.

The only thing I wasn't 100% sure in my patch is the volume update: self.sampler.update(vol / cur_nblive) whether I should have used cur_nblive or nblive

joshspeagle commented 3 years ago

Thanks for catching this and for the PR. Happy to merge this in. And I believe cur_nblive is probably the right thing to use there for the point mass of an individual point.