-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add ability to do vector-vector loop #75
Conversation
BREAKING CHANGE: this is very much a breaking change. The biggest API-facing change is that the output is now (Ntime, Nfeed, Nfeed, Npairs) instead of (Ntime, Nfeed, Nfeed, Nant, Nant). However, other additions are the inclusion of the choice of different methods for performing the matrix product.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #75 +/- ##
===========================================
+ Coverage 63.95% 97.82% +33.86%
===========================================
Files 8 21 +13
Lines 566 828 +262
Branches 88 98 +10
===========================================
+ Hits 362 810 +448
+ Misses 196 9 -187
- Partials 8 9 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for a hard work @steven-murray . I have some comments and small suggestions, just for clarification and aesthetic. I didn't try to read into the algorithm too much -- I believe you already tested and think through it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These look good now @steven-murray . Thanks again for your efforts. Thanks for cleaning up some unused functions that I have missed in the review as well.
setup.cfg
Outdated
@@ -21,6 +21,7 @@ packages = find_namespace: | |||
install_requires = | |||
astropy | |||
click | |||
docstring-parser | |||
line-profiler | |||
numpy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we ready to pin numpy>=2.0.0
and pyuvdata>=3.0.0
?
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
This adds the ability to do the matrix-matrix calculation instead as a targeted set of vector-vector dot products for only the pairs we care about.