Skip to content
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

Support broadcasting over structured block matrices #53909

Merged
merged 5 commits into from
Apr 7, 2024

Conversation

jishnub
Copy link
Contributor

@jishnub jishnub commented Mar 31, 2024

Fix #48664

After this, broadcasting over structured block matrices with matrix-valued elements works:

julia> D = Diagonal([[1 2; 3 4], [5 6; 7 8]])
2×2 Diagonal{Matrix{Int64}, Vector{Matrix{Int64}}}:
 [1 2; 3 4]           
            [5 6; 7 8]

julia> D .+ D
2×2 Diagonal{Matrix{Int64}, Vector{Matrix{Int64}}}:
 [2 4; 6 8]           
            [10 12; 14 16]

julia> cos.(D)
2×2 Matrix{Matrix{Float64}}:
 [0.855423 -0.110876; -0.166315 0.689109]  [1.0 0.0; 0.0 1.0]
 [1.0 0.0; 0.0 1.0]                        [0.928384 -0.069963; -0.0816235 0.893403]

Such operations show up when using BlockArrays.

The implementation is a bit hacky as it uses 0I as the zero element in fzero, which isn't really the correct zero if the blocks are rectangular. Nonetheless, this works, as fzero is only used to determine if the structure is preserved.

Copy link
Member

@dkarrasch dkarrasch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Should we perhaps have some failing tests, like matrices of matrices whose elements don't match in size, or something?

@jishnub
Copy link
Contributor Author

jishnub commented Apr 4, 2024

I'm actually struggling to come up with an example that specifically fails despite this, and isn't related to the properties of the elements (or missing UniformScaling methods). Perhaps I'll add some tests that are broken due to missing methods? These would hopefully be fixed in the future.

@dkarrasch
Copy link
Member

I was thinking of something like

A = Diagonal([zeros(2,3) for _ in 1:3]) # or zeros(3,3)
B = Diagonal([zeros(2,2) for _ in 1:3])
@testthrows SomeError A .+ B

@jishnub jishnub force-pushed the jishnub/blockstructuredbcast branch from 854c779 to 02924c8 Compare April 5, 2024 05:34
@jishnub
Copy link
Contributor Author

jishnub commented Apr 5, 2024

I've added a test with incompatible element sizes, and also one that is directly related to this PR's changes:

julia> D = Diagonal([ones(1,1), ones(1,1)])
2×2 Diagonal{Matrix{Float64}, Vector{Matrix{Float64}}}:
 [1.0;;]        
         [1.0;;]

julia> size.(D)
ERROR: MethodError: no method matching size(::UniformScaling{Float64})
The function `size` exists, but no method is defined for this combination of argument types.
[...]

Thankfully, this isn't a regression, as the operation didn't work before either. In fact, the following works now:

julia> ndims.(D)
2×2 Matrix{Int64}:
 2  2
 2  2

@jishnub jishnub merged commit 243ebc3 into master Apr 7, 2024
5 of 7 checks passed
@jishnub jishnub deleted the jishnub/blockstructuredbcast branch April 7, 2024 14:22
KristofferC pushed a commit that referenced this pull request May 13, 2024
@jishnub jishnub restored the jishnub/blockstructuredbcast branch May 14, 2024 05:16
@jishnub jishnub deleted the jishnub/blockstructuredbcast branch May 14, 2024 05:40
jishnub added a commit that referenced this pull request May 17, 2024
…54460)

This was reverted in #54332. This
needs #54459 for the tests to
pass. Opening this now to not forget about it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:arrays [a, r, r, a, y, s] domain:linear algebra Linear algebra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avoid explicitly evaluating zero elements in certain structured matrix broadcast operations
2 participants