joshspeagle / dynesty

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

Incorrect covariance scaling ? #222

Closed segasai closed 3 years ago

segasai commented 3 years ago

Looking at this line https://github.com/joshspeagle/dynesty/blob/8cfb26cde16e067a8d79cee0479b12936cbe09fc/dynesty/bounding.py#L1324 I see scaling of the covariance of the ellipsoid by (ndim+2) to adjust for the sample covaiance bias, but, as far as I can see, later on in the code https://github.com/joshspeagle/dynesty/blob/8cfb26cde16e067a8d79cee0479b12936cbe09fc/dynesty/bounding.py#L1375 we scale the matrix to just about include the points. So therefore the (ndim+2) scaling essentially goes away. If I understand correctly (ndim+2) needs to happen after the last rescaling (if it is needed at all).

joshspeagle commented 3 years ago

Yes, it looks like this rescaling is actually totally unnecessary and could likely be removed.

segasai commented 3 years ago

For the moment the scaling is no-op (and could be removed without any changes), but the question is whether there is an appropriate volume enhancement somewhere else in the code.

joshspeagle commented 3 years ago

I’d have to dig through, but I don’t actually think so (which is why the behavior was so sensitive to numerical instability here at least). There are volume changes based on values passed by the user that should kick in later I believe.

joshspeagle commented 3 years ago

Closing this since with all the bounding ellipsoid changes in the recent PRs this should essentially be a non-issue now.