search

joshspeagle / dynesty

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

Slice sampling bug (sampling appears stuck error) #250

Closed segasai closed 3 years ago

segasai commented 3 years ago

While trying to use rslice sampling, even with many fixes I still hit the problem of 'Slice sampling appears to be stuck'

  /home/skoposov/science/ting_streams/distance_tools.py(402)sampler_dyn()
-> dsampler.run_nested(
  /home/skoposov/pyenv38/lib/python3.8/site-packages/dynesty/dynamicsampler.py(1582)run_nested()
-> passback = self.add_batch(nlive=nlive_batch,
  /home/skoposov/pyenv38/lib/python3.8/site-packages/dynesty/dynamicsampler.py(1702)add_batch()
-> for results in self.sample_batch(nlive_new=nlive,
  /home/skoposov/pyenv38/lib/python3.8/site-packages/dynesty/dynamicsampler.py(1141)sample_batch()
-> for it, results in enumerate(
  /home/skoposov/pyenv38/lib/python3.8/site-packages/dynesty/sampler.py(773)sample()
-> u, v, logl, nc = self._new_point(loglstar_new, logvol)
  /home/skoposov/pyenv38/lib/python3.8/site-packages/dynesty/sampler.py(385)_new_point()
-> u, v, logl, nc, blob = self._get_point_value(loglstar)
  /home/skoposov/pyenv38/lib/python3.8/site-packages/dynesty/sampler.py(369)_get_point_value()
-> self._fill_queue(loglstar)
> /home/skoposov/pyenv38/lib/python3.8/site-packages/dynesty/sampler.py(358)_fill_queue()
-> self.queue = list(self.M(evolve_point, args))
  /home/skoposov/pyenv38/lib/python3.8/site-packages/dynesty/sampling.py(753)sample_rslice()
-> raise RuntimeError("Slice sampling appears to be "

The debugging tells me what happens is that the new loglikelihood level is selected https://github.com/joshspeagle/dynesty/blob/c22158cfb1a6bc740f7add2db9d238e272a9dbf6/py/dynesty/sampler.py#L763 Then the new point satistfying the L>Lmin value is selected here https://github.com/joshspeagle/dynesty/blob/c22158cfb1a6bc740f7add2db9d238e272a9dbf6/py/dynesty/sampler.py#L773 The problem is that for the rslice sampling the _new_point will take one of the current livepoints (using propose_live) (potentially the one NOT satisfying the L>Lmin condition) and do slice sampling from there which can fail easily (depending on the topology of level contours)

joshspeagle commented 3 years ago

This seems like a straightforward fix to how propose_live works, where it just checks that the live point that is chosen for the first slice has L > Lmin. Thanks for catching this.

segasai commented 3 years ago

Yes, it seems like that's the appropriate fix. I only wasn't sure if propose_live is used in other contexts to indeed generate a random live points, and whether therefore a new method has to be written.

joshspeagle commented 3 years ago

I just double-checked that this should be fine -- it doesn't appear to be used in any other context outside of proposals. I had forgotten how convoluted some of the internal structure is, but the method is defined separately in each NestedSampler class and then gets aliased as propose_point and called exclusively from here as far as I can tell.

The syntax there already has loglstar defined in the preceding condition, so it looks like adding loglstar as an (optional?) argument, then modifying all of the propose_live (and propose_unif) functions to accept the new syntax, should probably work.

segasai commented 3 years ago

I looked at it this morning and wasn't comfortable making this change. I also didnt' want to add another logl() call at the position of live points (as those should be already available). Also, the contracts for the propose_live, propose_points were not clear to me. I.e. what exactly they are doing and what they are guaranteeing across different samplers.

joshspeagle commented 3 years ago

Yea, I'm happy to push this change. The logl values for all the live points are stored internally, so agreed those should be easy to access without any additional likelihood calls.

The specific structure here was created with the possibility that I could swap out proposal and sampling methods depending on the context. In practice, it ended up being an alias for all sampling methods except uniform rejection sampling.

joshspeagle commented 3 years ago

With the dynamic sampling issue resolved, should I take a look at trying to implement this fix either tonight/tomorrow?

segasai commented 3 years ago

Yes, that would be great. Hopefully after that the slice sampling will be much more usable. (as one can find other reports of similar issues in the past like #208 )