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

Likelihood should be strictly increasing? #202

Closed ColmTalbot closed 4 years ago

ColmTalbot commented 4 years ago

Hi @joshspeagle I came across some weird behaviour which I think is down to dynesty accepting new points based on the condition logl >= loglstar, e.g., https://github.com/joshspeagle/dynesty/blob/master/dynesty/sampler.py#L393.

My understanding of the NS algorithm was that the likelihood needs to be strictly increasing, I think that this means the correct condition is logl > loglstar.

joshspeagle commented 4 years ago

Oh Y I K E S -- yes, it should be strictly increasing. That's a nasty typo. If you'd be willing to push a quick PR correcting those if they appear in more than one place that'd be much appreciated. Otherwise I'll definitely revise that condition in the next commit.

ColmTalbot commented 4 years ago

Done, https://github.com/joshspeagle/dynesty/pull/203. I verified that this fixes the issue I saw.

joshspeagle commented 4 years ago

I'll merge that in ASAP. Thanks!

ColmTalbot commented 4 years ago

Thanks for merging so quickly. Do you have any plans to do a new release in the near future? It would be great to have this fix in a released version.

joshspeagle commented 4 years ago

Yes, I think there's now enough improvements in the dev version (and enough "small fixes" that I've flagged) that I should try to clean things up and push out a new release. I'll aim for putting something out by the end of the month, although if you'd like things sooner please let me know.