Conversation
Codecov Report
@@ Coverage Diff @@
## main #81 +/- ##
==========================================
+ Coverage 88.62% 88.72% +0.09%
==========================================
Files 13 13
Lines 554 550 -4
==========================================
- Hits 491 488 -3
+ Misses 63 62 -1
Continue to review full report at Codecov.
|
|
Pushed the second commit that tries to make a proper fix. |
|
Hey @aplavin thanks for the work on this, sorry I haven't had time to respond to any of this yet, I'm swamped under my thesis work nowadays. I've been wanting to rewrite the bounding space algorithms for a while now, I think they're the slowest part of the NS code (not sure the impact on total performance, but the fits have tons of allocations and use mutable structs and are pretty haphazard). I don't really like the interface, either, and I think it can all be rewritten to utilize Distributions.jl since these "bounding spaces" are just statistical distributions. So, with all that being said, I just want you to know that I've seen this, and that I have ideas for making this part of the interface better! I really appreciate what you've contributed already 😃 |
|
Yeah, no real hurry. |
upd: this post describes background and the first commit of this PR
It's probably more reasonable? But still not a complete fix, see below.
I encountered an error arising from the
Amatrix here having (kinda) negative eigenvalues: https://github.com/TuringLang/NestedSamplers.jl/blob/eb6999e0da004524a5848eb2c99b1f53e8a57f1d/src/bounds/ellipsoid.jl#L108This matrix is passed to the
Ellipseconstructor, and fails insqrtat https://github.com/TuringLang/NestedSamplers.jl/blob/eb6999e0da004524a5848eb2c99b1f53e8a57f1d/src/bounds/ellipsoid.jl#L49Actually, eigenvalues of
Aare not negative as-is, but become negative inSymmetric(A). Digging into this error, I found that the https://github.com/TuringLang/NestedSamplers.jl/blob/eb6999e0da004524a5848eb2c99b1f53e8a57f1d/src/bounds/ellipsoid.jl#L168-L178function applies a huge correction to the cov matrix in my case at https://github.com/TuringLang/NestedSamplers.jl/blob/eb6999e0da004524a5848eb2c99b1f53e8a57f1d/src/bounds/ellipsoid.jl#L105
Not sure why it is so, and I don't understand the
make_eigvals_positive!function, what exactly should it do and why? Maybe, adapting the function https://github.com/joshspeagle/dynesty/blob/2c5f1bbe5a0745c6625876f23ec6aa710c845fd4/py/dynesty/bounding.py#L1230 from dynesty would be better?This PR fixes the error with the particular matrix (below) because it has the condition number < 1e10. But I don't think it's a proper solution.
To reproduce my original error in
fit():