joshspeagle / dynesty

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

fix a factor of 2 in the BIC calculation #442

Closed segasai closed 1 year ago

segasai commented 1 year ago

This is a tentative fix to the BIC criterion for ellipsoid splitting introduced in #286 There a factor of two was skipped in the calculation making the splitting of ellipsoids harder. Apparently that leads to significant slowness observed in #440

I'm a bit torn by this issue and whether it's right change. One wouldn't want the results to be that sensitive to that factor of two here and I'm concerned if this will break some other cases...

One thing that this change trips is the test_ellipsoids test case https://github.com/joshspeagle/dynesty/blob/d9164354421f4a010ee2bc232cb4eda2f4f90a88/tests/test_ellipsoid.py#L265 where I check if the number of clusters in 6 x 6 x 6 x 6 grid in 4d is within 10% of 6^4. I don't know if it's a serious concern or not...

Thoughts ?

coveralls commented 1 year ago

Pull Request Test Coverage Report for Build 5018080593


Totals Coverage Status
Change from base Build 5017942422: 0.002%
Covered Lines: 4162
Relevant Lines: 4527

💛 - Coveralls