joshspeagle / dynesty

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

Add additional rwalk proposals #366

Open ColmTalbot opened 2 years ago

ColmTalbot commented 2 years ago

This PR adds a bunch of different methods for proposing points with the rwalk sampling.

These are a combination of different scale distributions for the existing method and ensemble moves.

Some of the unit tests are failing, I still need to dig into that.

cc @joshspeagle @segasai

coveralls commented 2 years ago

Pull Request Test Coverage Report for Build 2173142774


Changes Missing Coverage Covered Lines Changed/Added Lines %
py/dynesty/proposals.py 73 74 98.65%
py/dynesty/sampling.py 45 46 97.83%
<!-- Total: 144 146 98.63% -->
Totals Coverage Status
Change from base Build 2157424611: 0.2%
Covered Lines: 3767
Relevant Lines: 4169

💛 - Coveralls
segasai commented 2 years ago

Thanks for the PR.

In my opinion I would prefer to start merging just the ensenble sampling part and leave outside the axis based proposals and chi-proposals (unless there is a clear demonstrable benefit).
Also I think we have to be careful with the naming in the patch 'proposals', 'ellipsoidal_proposals' to avoid confusion with uniform ellipsoidal sampling etc. (at the moment there are also various names used by the existing sampler: we start by proposing the point (usually one of the live points), we then evolve the point; the last bit is done by various slice/walk samplers; I think some of those names are not great, but should try to make sure that things are consistently named so we don't confuse ourselves)

For the ensemble sampling part, it seems that there is an easy to write example of a multimodal distribution where it'd be beneficial. I think it'd be good to put that example here, so we can assess the improvement/test that it perfoms like it should. (IMO the test suite is mostly checking that things can run and are not outright broken, rather testing the efficiency)

segasai commented 2 years ago

I have spent some time looking and thinking about this proposal, and my current thinking (on top of what I said previously) is that

We would need some kind of interface for those samplers. Right now the samplers essentially have propose_point, evolve_point methods, but that's probably too restrictive for example for the ensemble sampler. In fact it's unclear if we want to keep the separation between propose and evolve.

As far I can see the classes defined here https://github.com/joshspeagle/dynesty/blob/master/py/dynesty/nestedsamplers.py that are boundary specific will need to take as an argument classes that will need to do sampling operations like: Take the list of points subject to L>Lconstraint, (optional boundary specification) -> propose a new point (or several) subject to constraint (L>L_)

as far as I can see that would allow us more clearly add new samplers (and have them implemented externally even), and avoid adding more options for NestedSampler.

Thoughts ?