-
Notifications
You must be signed in to change notification settings - Fork 42
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 Kronecker products for LinearMaps #61
Conversation
Codecov Report
@@ Coverage Diff @@
## master #61 +/- ##
==========================================
- Coverage 98.83% 98.24% -0.59%
==========================================
Files 8 9 +1
Lines 428 456 +28
==========================================
+ Hits 423 448 +25
- Misses 5 8 +3
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #61 +/- ##
==========================================
- Coverage 97.49% 96.42% -1.07%
==========================================
Files 8 10 +2
Lines 479 616 +137
==========================================
+ Hits 467 594 +127
- Misses 12 22 +10
Continue to review full report at Codecov.
|
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.
Code looks fine to me.
One could easily add a method for the product of two Kronecker maps,
returning yet another Kron. map.
I saw that in Wikipedia, but I thought that works only when the matrix products exists, but they don't have to...? |
Hooray, I managed to add promotion of matrices (as long as there is one |
What I have in mind would inherit the dimension checks already there in making a From wikipedia, the "mixed-product property:" Without thinking about types (as usual for me :) ) I think it is almost a one liner (lazy rough sketch; should use
Ah, but type instability... Again my use case for this is in the preamble before iterating so I don't think that instability would matter. |
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.
Impressive metaprogramming but it is hard for me to follow. A couple comments would really help!
But I really don't understand how this works with the multiply with vector.
The vector multiply as just A,B = L.maps so what happens with 3 or more maps?
I don't know how the vec trick generalizes to beyond 2, though perhaps it can with some type of recursion. Again, maybe a comment in the code to explain would help?
In terms of numerical effort, I assume it would be beneficial to use that mixed-property? Could be that this would be a second "good" use case for the union splitting mechanism. The promotion thing is really "simple". All it does is to take the matrix arguments and wraps them in a I'll add some comments to the code. |
I just did a quick check, and in fact it's beneficial to use the mixed-trick. I'll put it in the PR, then we discuss the stability issue. |
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.
looks great!
I thought I'll post a little comparison here for general entertainment. I'm sure I have used using BenchmarkTools, Test
using LinearAlgebra, SparseArrays
using LinearMaps, Kronecker, LazyArrays
n = 100
x = rand(n*n)
y = zeros(n*n)
@info "LinearMaps.jl"
J = LinearMaps.UniformScalingMap(true, n)
E = spdiagm(-1 => trues(n-1))
D = E + E' - 2I
A1 = kron(D, J) + kron(J, D)
@btime $A1 * $x
@info "Kronecker.jl"
J = I(n)
A2 = kronecker(D, J)
A2´ = kronecker(J, D)
@btime $A2 * $x + $A2´ * $x
@info "LazyArrays.jl"
A3 = Kron(D, J)
A3´ = Kron(J, D)
@btime $A3 * $x + $A3´ * $x
@info "Base"
A4 = kron(D, J) + kron(J, D)
@btime $A4 * $x
@test sparse(A1) == A4 and here is what I got: [ Info: LinearMaps.jl
956.586 μs (726 allocations: 521.30 KiB)
[ Info: Kronecker.jl
5.115 ms (64 allocations: 1.35 MiB)
[ Info: LazyArrays.jl
5.771 s (6 allocations: 234.61 KiB)
[ Info: Base
55.835 μs (2 allocations: 78.20 KiB) This is the discrete Laplace operator. Note that with One thing I noted is that there doesn't seem to be a "lazy" version of a |
Nice timing results! I didn't know about |
Yeah, I didn't know which other symbol to use so I typed some random symbol. 😛 Well, and yes, the array type behavior is what distinguishes those packages from ours here. Some day you'll have to explain me why you can't live without |
a569922
to
f6d9d20
Compare
Regarding the minimal storage but known-size uniform scaling object, I just found this discussion here: JuliaLang/julia#30298 The use of LinearMap(J::UniformScaling, n::Int) = UniformScalingMap(J.λ, n) and then we don't need to export |
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.
Thumbs up for Kronecker sum!
Thanks, and mat-vec-multiplication with Kronecker products is also faster now. I reuse intermediate vectors (called |
09f9050
to
c8ac052
Compare
This is still WIP, basically it needs some more tests, but I wanted to quickly share some impressive results, obtained on Julia v1.3 with the 5-arg using BenchmarkTools, Test
using LinearAlgebra, SparseArrays
using LinearMaps, Kronecker#, LazyArrays
n = 100
x = rand(n*n)
y = zeros(n*n)
@info "LinearMaps.jl"
J = LinearMap(I, n);
E = spdiagm(-1 => trues(n-1));
D = E + E' - 2I;
L = kron(D, J) + kron(J, D);
@btime $L * $x; # 89.378 μs (27 allocations: 313.48 KiB)
@info "Kronecker.jl"
J = Kronecker.Eye(n);
A2 = kronecker(D, J);
A2´ = kronecker(J, D);
@btime ($y = $A2 * $x; $y.+= $A2´ * $x); # 89.393 μs (17 allocations: 313.20 KiB)
@info "Base"
J = I(n);
A4 = kron(D, J) + kron(J, D);
@btime $A4 * $x; # 57.114 μs (2 allocations: 78.20 KiB) We are getting really close to the super-sparse matrix multiplication. Interestingly, the operator is nothing but a Kronecker sum. J = LinearMap(I, n);
E = spdiagm(-1 => trues(n-1)); D = E + E' - 2I; DL = LinearMap(D);
Δ₂ = LinearMaps.kronsum(DL, DL);
Δ₃ = kroneckersum(D, D);
@btime $Δ₂ * $x; # 67.537 μs (7 allocations: 78.41 KiB)
@btime $Δ₃ * $x; # 67.327 μs (7 allocations: 78.41 KiB) So, we are able to fully leverage all |
This has grown big, as usual, but I think it's ready for review. For Kronecker products of matrix-based linear maps to work fast, there were quite some adjustments needed for |
9bacbdc
to
e701ff5
Compare
Brief update: I made the Kronecker (sum) power a type parameter, so this way everything is inferable. This puts a little overhead onto the compiler, but I guess this is not going to be used with many different powers. |
Hi @dkarrasch , could you remind me again which open PRs are finalised and ready for review; I keep forgetting and was very busy the last weeks. |
It would be good to get this one done here. |
Sorry for messing around. When I'm bored, I'm starting to do micro-optimization. 😂 I've now optimized dimension checks, and included the option to skip them (as usual) via Okay, I promise I'll leave this PR in peace for review. 🤞 But actually, the recent changes are just standard refactors of the previous code, no design changes or anything like that. |
gentle bump |
I tried to complete the review now but I am too tired; this time I will try not to forget. |
Generally looks good; I think this can be merged, maybe some small comments for consideration. |
I realized that part of @JeffFessler's motivation for having
LinearMaps <: AbstractArray
is to use them in combination with Kronecker.jl. I thought I'll lower the pressure and provide a Kronecker product forLinearMap
s without the need forgetindex
. 😛The implementation uses the well-known vec trick, and since the Kronecker product is associative, one only needs to take care of the product of two linear maps. I couldn't find any trick for "higher-order" Kronecker products. Note that this is all lazy, and all linear map types are, as usual, inferred. This works out of the box with wrapped maps of dense/sparse/structured matrices, or function maps, etc.