nanograv / enterprise

ENTERPRISE (Enhanced Numerical Toolbox Enabling a Robust PulsaR Inference SuitE) is a pulsar timing analysis code, aimed at noise analysis, gravitational-wave searches, and timing model analysis.
https://enterprise.readthedocs.io
MIT License
64 stars 65 forks source link

Allow for index arrays in ShermanMorrison #356

Closed vhaasteren closed 9 months ago

vhaasteren commented 11 months ago

This one solves #319 by allowing EcorrKernelNoise to work with index arrays, rather than slice objects. The function that is changed now is quant2ind, which now by default returns an list of index arrays.

The fastshermanmorrison package with Cython code is also compatible with this change, in case anyone is using that. Just upgrade it with pip

Thanks to Bjorn Larsen for providing a first outline of this modification.

TODO:

Depends on #352 for codev report

codecov[bot] commented 11 months ago

Codecov Report

Merging #356 (b9a3bcc) into dev (2415762) will increase coverage by 0.07%. The diff coverage is 96.66%.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/nanograv/enterprise/pull/356/graphs/tree.svg?width=650&height=150&src=pr&token=7Sjk8cLA85&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nanograv)](https://app.codecov.io/gh/nanograv/enterprise/pull/356?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nanograv) ```diff @@ Coverage Diff @@ ## dev #356 +/- ## ========================================== + Coverage 88.35% 88.42% +0.07% ========================================== Files 13 13 Lines 3023 3033 +10 ========================================== + Hits 2671 2682 +11 + Misses 352 351 -1 ``` | [Files](https://app.codecov.io/gh/nanograv/enterprise/pull/356?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nanograv) | Coverage Δ | | |---|---|---| | [enterprise/signals/utils.py](https://app.codecov.io/gh/nanograv/enterprise/pull/356?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nanograv#diff-ZW50ZXJwcmlzZS9zaWduYWxzL3V0aWxzLnB5) | `86.77% <100.00%> (+0.31%)` | :arrow_up: | | [enterprise/signals/white\_signals.py](https://app.codecov.io/gh/nanograv/enterprise/pull/356?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nanograv#diff-ZW50ZXJwcmlzZS9zaWduYWxzL3doaXRlX3NpZ25hbHMucHk=) | `98.62% <100.00%> (+0.02%)` | :arrow_up: | | [enterprise/signals/signal\_base.py](https://app.codecov.io/gh/nanograv/enterprise/pull/356?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nanograv#diff-ZW50ZXJwcmlzZS9zaWduYWxzL3NpZ25hbF9iYXNlLnB5) | `90.14% <94.44%> (+0.03%)` | :arrow_up: | ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/nanograv/enterprise/pull/356?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/356?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nanograv). Last update [2415762...b9a3bcc](https://app.codecov.io/gh/nanograv/enterprise/pull/356?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).
vhaasteren commented 9 months ago

@AaronDJohnson, unit tests for the extra lines are done. As commented on the code, the remaining two lines that aren't covered have never been covered and IMHO shouldn't even exist. I vote to leave those like this