joshspeagle / dynesty

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

Batch fix #313

Closed segasai closed 3 years ago

segasai commented 3 years ago

Fix of #311 and #302

coveralls commented 3 years ago

Pull Request Test Coverage Report for Build 1160713084


Changes Missing Coverage Covered Lines Changed/Added Lines %
py/dynesty/dynamicsampler.py 15 16 93.75%
<!-- Total: 15 16 93.75% -->
Files with Coverage Reduction New Missed Lines %
py/dynesty/dynamicsampler.py 1 88.69%
<!-- Total: 1 -->
Totals Coverage Status
Change from base Build 1157451312: 0.01%
Covered Lines: 3456
Relevant Lines: 4255

💛 - Coveralls
segasai commented 3 years ago

Okay, this is a rehash of https://github.com/joshspeagle/dynesty/issues/244 apparently. So my fix is incorrect, because we need to update logl_min, logl_max otherwise we'll fail to sample when some of our live points are below the threshold. Also, I realized that the weighting from here https://github.com/joshspeagle/dynesty/issues/244 which was coded to be 1/L_i 1/W_i ( see also https://github.com/joshspeagle/dynesty/pull/248#issuecomment-845250127 ) is wrong.. If I understand correctly now weighting need to be X_i, i.e we need to downweight proportionally to volume occupied.

segasai commented 3 years ago

I've tentatively addressed the issue. I've fixed the weights to be volumes. And if there are not enough points above the boundary I move the boundary. (updating the logl_min of the run). That rubs me the wrong way somewhat, because we are updating the inputs given to us, but maybe that's fine. Maybe we need to give the warning in this case. Also, apparently in the fix of the padding inside weight_function that I've done before (when if there is only one point on the right edge, I didn't expand it as correctly to the left ( that's why my very first hacky fix of #302 didn't work)

joshspeagle commented 3 years ago

Okay, this looks good. I think the combination of the weight update and the more robust logic should hopefully now address the edge case where sampling terminates really early.