New ecorr sqrtsolve#8
Merged
Merged
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8 +/- ##
==========================================
- Coverage 92.42% 91.76% -0.66%
==========================================
Files 2 2
Lines 198 255 +57
==========================================
+ Hits 183 234 +51
- Misses 15 21 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
This was referenced Feb 20, 2026
This was referenced Apr 30, 2026
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sqrtsolveimplementation that computes N^{-1/2} X using a closed-form diagonal-plus-rank-1 inverse square-root identity, replacing the previous Cholesky-based approach as the defaultcholsqrtsolvefor all three classes (ShermanMorrison,FastShermanMorrison, and the test reference class)sqrtsolveis ~2.4x faster thancholsqrtsolvefor the whitening step, and computing Z^T N^{-1} Z viasqrtsolve+ W^T W is competitive with (or slightly faster than) the direct BLAS-accelerated Sherman-Morrison 2D solveDetails
Mathematical approach: For each ECORR block with covariance D + j 11^T, the new sqrtsolve uses the rank-1 identity
(I + u u^T)^{-1/2} x = x + alpha u (u^T x), alpha = -1 / (sqrt(1+v) (sqrt(1+v) + 1))
where v = ||u||^2. This avoids forming or solving a triangular Cholesky factor entirely. The alpha formula is the numerically stable equivalent of (1/sqrt(1+v) - 1)/v, avoiding cancellation for all values of v.
What changed:
python_block_sqrtsolve_rank1/python_idx_sqrtsolve_rank1: new pure-Python implementations using the closed-form formulacython_block_sqrtsolve_rank1/cython_idx_sqrtsolve_rank1: new Cython implementations of the same*_cholsqrtsolve_*(all four variants: python block/idx, cython block/idx)ShermanMorrisonandFastShermanMorrisonclasses now expose bothsqrtsolve()(non-Cholesky, default) andcholsqrtsolve()(Cholesky-based) public methods_sqrtsolve_D2on all classes now calls the non-Cholesky path;_cholsqrtsolve_D2preserves the old behaviorTests:
test_sqrtsolve_D12updated to test the new non-Cholesky sqrtsolve (element-wise cross-checks between Python reference and Cython, plus end-to-end Z^T N^{-1} Z = (WZ)^T (WZ) verification)test_cholsqrtsolve_D12added for the Cholesky-based path, with full cross-validation across all three implementations (Python rank-1, scipy Cholesky, Cython rank-1) and the same end-to-end correctness checkBenchmark (not included in codebase)
48,000 TOAs (1500 epochs x 32 obs/epoch), 500 parameters:
Numerical accuracy identical across all paths (~2e-15 relative error).
Test plan
test_solve_D1,test_solve_1D1,test_solve_2D2,test_sqrtsolve_D12,test_errors,test_logdet)test_cholsqrtsolve_D12passes