Faster sparse matrix Cholesky

[vhaasteren]

This PR changes the way the sparse matrix Cholesky is carried out. The first time the likelihood is called, a regular

self.cf = cholesky(Sigma_sp)

is done. However, every subsequent call only the numerical decomposition is carried out. The analytical sparse decomposition takes a significant amount of time for our decompositions, so there is a speedup by just doing an 'update':

self.cf.cholesky_inplace(Sigma_sp)

For the NANOGrav 15yr set, this alone reduces the model_3A time on my laptop from 370ms to 280ms. Additional speedups might be possible in the construction and decomposition of 'phi'

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 88.44%. Comparing base (c1abead) to head (5c1e455). Report is 12 commits behind head on dev.

:exclamation: Current head 5c1e455 differs from pull request most recent head 1a5d2e4. Consider uploading reports for the commit 1a5d2e4 to get more accurate results

Additional details and impacted files

[](https://app.codecov.io/gh/nanograv/enterprise/pull/376?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nanograv) ```diff @@ Coverage Diff @@ ## dev #376 +/- ## ========================================== + Coverage 88.43% 88.44% +0.01% ========================================== Files 13 13 Lines 3034 3037 +3 ========================================== + Hits 2683 2686 +3 Misses 351 351 ``` | [Files](https://app.codecov.io/gh/nanograv/enterprise/pull/376?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nanograv) | Coverage Δ | | |---|---|---| | [enterprise/signals/signal\_base.py](https://app.codecov.io/gh/nanograv/enterprise/pull/376?src=pr&el=tree&filepath=enterprise%2Fsignals%2Fsignal_base.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nanograv#diff-ZW50ZXJwcmlzZS9zaWduYWxzL3NpZ25hbF9iYXNlLnB5) | `90.18% <100.00%> (+0.03%)` | :arrow_up: | ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/nanograv/enterprise/pull/376?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nanograv). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nanograv) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/nanograv/enterprise/pull/376?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nanograv). Last update [b1b25bb...1a5d2e4](https://app.codecov.io/gh/nanograv/enterprise/pull/376?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nanograv). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nanograv).

[vallis]

Looks ok. Perhaps it would be good to add a test that the results are invariant? Call the likelihood for two parameter sets, then erase the cached cholesky and do it again for the second parameter set.

Also, what's going on with the Mac OS tests?

[AaronDJohnson]

It looks like the Mac builds failed to install suitesparse @vallis. I'm rerunning one of them to see if it's on their end.

[vhaasteren]

One of the headers couldn't be found (cholmod.h), so perhaps they repackaged it? There were major modifications to the build system it says in the changelog.

Anyway, good idea regarding adding the extra tests. Once I figure out how to erase the cache I'll add those to this PR

[AaronDJohnson]

Rerun these tests after merging the new dev changes to fix Mac CI :)

[vhaasteren]

Great, they pass. I'll still add the tests though, so don't merge yet

[vhaasteren]

Ok, I finally wrote the check @AaronDJohnson. This is ready for review/merge