joshspeagle / dynesty

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

update error message if trying to get bounding ellipse of a single point #305

Closed segasai closed 3 years ago

segasai commented 3 years ago

Helps #302

coveralls commented 3 years ago

Pull Request Test Coverage Report for Build 1148893105


Changes Missing Coverage Covered Lines Changed/Added Lines %
py/dynesty/sampler.py 7 8 87.5%
py/dynesty/bounding.py 13 15 86.67%
py/dynesty/sampling.py 5 7 71.43%
py/dynesty/dynamicsampler.py 57 61 93.44%
py/dynesty/utils.py 59 64 92.19%
<!-- Total: 153 167 91.62% -->
Files with Coverage Reduction New Missed Lines %
py/dynesty/dynamicsampler.py 1 87.2%
py/dynesty/dynesty.py 1 80.65%
py/dynesty/utils.py 2 83.51%
<!-- Total: 4 -->
Totals Coverage Status
Change from base Build 969454074: 0.3%
Covered Lines: 3606
Relevant Lines: 4477

💛 - Coveralls
segasai commented 3 years ago

Hi, This branch is ready to be merged (it has a collection of various things that'd be nice to merge) I want to bring to particular attention this commit 54e268eeeb2d0e1551dbf2ff30d4020583d92199 as it fixes an annoying bug that I've introduced a couple of months ago in the update_interval calculation logic.

joshspeagle commented 3 years ago

How is this looking? Can I looking into merging this in? I also want to push the fix to https://github.com/joshspeagle/dynesty/commit/54e268eeeb2d0e1551dbf2ff30d4020583d92199 that we missed and get the updated version on pip sooner rather than later.

segasai commented 3 years ago

I think this is good to merge, with the only concern over the factor of two in front of the variance calculation (see another issue). If you are happy with that, I think this can be merged. (there is one more function where I'd like to reuse the compute_integrals(), but I can do it in a separate commit later)

joshspeagle commented 3 years ago

Okay, then let me look into resolving that first, we can add it in, and then merge.

segasai commented 3 years ago

Great! Thanks