joshspeagle / dynesty

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

Hanging of the pathology test, volume scaling #282

Closed segasai closed 3 years ago

segasai commented 3 years ago

This

env PYTHONPATH=py:tests:$PYTHONPATH python  -c 'import numpy.random as R,test_pathology as T;R.seed(5);T.test_pathology_dynamic()'

(executed in the main dynesty directory)

will currently hang the test at this

7803it [00:02, 3350.77it/s, batch: 0 | bound: 17 | nc: 1 | ncall: 29661 | eff(%): 26.025 | loglstar:   -inf < 15.517 <    inf | logz:  2.672 +/-  0.138 | dlogz:
8125it [00:20, 3350.77it/s, batch: 1 | bound: 18 | nc: 161 | ncall: 29983 | eff(%): 27.099 | loglstar: 18.214 < 18.421 < 18.421 | logz:  2.868 +/-  0.434 | stop:  2.398]    
.....

A quick check reveals that this is related to scale_to_logvol leading to linear rescaling of up to 1000 of the ellipsoid which then leads to the difficulties in finding the point in such ellipsoid.

I don't have a fix ATM. I think the pointvol logic is certainly questionable (or unclear to me). Alternatively some additional checks/logic can to be put in when ellipsoid is rescaled to many times the size of the unit cube.

joshspeagle commented 3 years ago

I think we'd discussed this a bit before re: putting a ceiling on the maximum axes length based on the length of the diagonal of the unit cube \sqrt(n), although nothing's been put in quite yet.

segasai commented 3 years ago

There is a limit now for the slice sampler (I've implemented it in one of the last patches), because there is a natural scale there. But this problem happens with the uniform (within ellipsoids) sampler. (and I still think that the pointvol assignment may need to be rethought as that's the root cause IMO, or alternatively the volume calculation of ellipsoids longer than the cube need to be updated)

joshspeagle commented 3 years ago

Right. I think given how things are now, we can probably afford to put in two checks for this:

This should help to counteract some of the issues that can arise from pointvol, which is still not ideal but at least a solution for the case where you end up with a bound centred on a single point. (If that edge case is successfully resolved elsewhere, I think we can safely remove it from all other locations in the code.)

segasai commented 3 years ago

The #283 and #284 resolve this issue.