-
Notifications
You must be signed in to change notification settings - Fork 536
RF: Rearranging matmul order and using hermitian flag in ICC_rep_anova #3453
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
Conversation
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.
Nice! I ran some quick tests just to see what this looks like. The reordering seems to dominate, but both changes seem well worth making.
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.
For a simulated Y of shape 50 x 10, I get:
In []: %timeit dot(dot(dot(X, pinv(dot(X.T, X))), X.T), Y.flatten("F")) 1.42 ms ± 128 μs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)
Convert to matmul:
In []: %timeit (X @ pinv(X.T @ X) @ X.T) @ Y.flatten("F") 1.4 ms ± 188 μs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)
No change. Reorder multiplications:
In []: %timeit X @ (pinv(X.T @ X) @ (X.T @ Y.flatten("F"))) 838 μs ± 43.3 μs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)
41% speedup. Use hermetian optimization:
In []: %timeit X @ (pinv(X.T @ X, hermitian=True) @ (X.T @ Y.flatten("F"))) 785 μs ± 17.8 μs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)
6% relative speedup for total of 45%.
I think you are going to want to remove the dot import from the top of the file, since it's no longer used. Otherwise looks good to me.
Codecov Report
@@ Coverage Diff @@ ## master #3453 +/- ## ======================================= Coverage 65.24% 65.24% ======================================= Files 309 309 Lines 40833 40833 Branches 5376 5376 ======================================= Hits 26641 26641 Misses 13118 13118 Partials 1074 1074
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
I removed the numpy.dot import
Thanks!
Uh oh!
There was an error while loading. Please reload this page.
Summary
See #3450
List of changes proposed in this PR (pull-request)
Calculation of predicted_Y refactored such that: