-
Notifications
You must be signed in to change notification settings - Fork 11
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
add multiplication with vectors method #32
Conversation
Codecov Report
@@ Coverage Diff @@
## master #32 +/- ##
=========================================
+ Coverage 94.77% 94.9% +0.13%
=========================================
Files 4 4
Lines 153 157 +4
=========================================
+ Hits 145 149 +4
Misses 8 8
Continue to review full report at Codecov.
|
🎉 Thanks a lot! LGTM
That'd be a great follow-up PR, if you have time for that? For this PR, would you mind posting a simple benchmark giving a quick illustration of the speed-up (it'd be helpful when one day making proper benchmarks for the package #19) And please bump the package version -- will tag a new release straight away! |
Here is a quick benchmark, slightly adopted from the test suite: using BlockDiagonals, BenchmarkTools
N1, N2, N3 = 30, 40, 50
N = N1 + N2 + N3
b1 = BlockDiagonal([rand(N1, N1), rand(N2, N2), rand(N3, N3)])
a = rand(N)
@benchmark $b1*$a Master: BenchmarkTools.Trial:
memory estimate: 4.41 MiB
allocs estimate: 43921
--------------
minimum time: 1.702 ms (0.00% GC)
median time: 1.852 ms (0.00% GC)
mean time: 2.195 ms (11.26% GC)
maximum time: 7.055 ms (44.17% GC)
--------------
samples: 2270
evals/sample: 1 This PR: BenchmarkTools.Trial:
memory estimate: 2.63 KiB
allocs estimate: 9
--------------
minimum time: 1.121 μs (0.00% GC)
median time: 1.227 μs (0.00% GC)
mean time: 1.633 μs (4.68% GC)
maximum time: 180.318 μs (97.38% GC)
--------------
samples: 10000
evals/sample: 10 |
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.
This is great! Thanks a lot. One tiny style comment, then good to go.
This seems to fix an oversight. Previously, there was no specialized
*(::BlockDiagonal,::AbstractVector)
method, such that this combination was handled by some genericLinearAlgebra
method. Not surprisingly, this yields a giant performance gain.I have kept the changes simple and minimal. There is quite some code duplication with the corresponding
*(::BlockDiagonal,::AbstractMatrix)
method, which can be reduced by means of a helper function and usingselectdim(y, 1, st:ed)
if desired.