joshspeagle / dynesty

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

Regularization refactor #228

Closed segasai closed 3 years ago

segasai commented 3 years ago

Another attempt to fix the problematic ellipsoids. Apparently it still didn't quite work (I had a problem that was consistently failing). The problem was that despite the fact that the covariance matrix was invertible and had positive eigen values, that still caused problem in the X^TAX normalization. The solution seems to be to start regularizing when condition number (ratio of eigen values) gets too high. I also moved the matrix regularization code in the special function. The cutoff values for the condition number of the matrix is still rather arbitrary, but it does solve my problem.

I also directly observed the effect of that function kicking in. Basically it slows things down reducing the acceptance rate, but the fit goes through. I was thinking about adding a warning if that code kicks in (which mean highly degenerate distribution), but I was worried about spamming the terminal, as each ellipsoid will make a warning, so I didn't do that.

joshspeagle commented 3 years ago

This is a good strategy for the fixes. I'm fine with the number being rather arbitrary. With regard to warnings, I've had the same considerations as well. If warnings will be thrown a number of times (but not all the time), I've generally been in favour of keeping things in. But if this is something that would be thrown constantly after a certain point, I would be in favour of just keeping it as is (without the warning).

segasai commented 3 years ago

It would be nice to merge this. The test failure is pylint 2.7 failure. And this is moot given the github actions PR. (it's also a bit hard to keep track when there are multiple unmerged branches.

joshspeagle commented 3 years ago

Yep, happy to merge this in.