From 997797d8ffdb12052d8844f349a9422cf61e7995 Mon Sep 17 00:00:00 2001 From: Matt Bauman Date: Wed, 10 Apr 2019 17:02:42 -0400 Subject: [PATCH 1/2] fix #31674, error when storing nonzeros into structural zeros with .= Previously, broadcasted assignment (`.=`) would happily ignore all nonstructured portions of the destination, regardless of whether the broadcasted expression would actually evaluate to zero or not. This changes these in-place methods to use the same infrastructure that out-of-place broadcast uses to determine the result type. If we are unsure of the structural properties of the output, we fall back to the generic implementation, which will attempt to store into every single location of the destination -- including those structural zeros. Thus we now error in cases where we generate nonzeros in those locations. --- .../LinearAlgebra/src/structuredbroadcast.jl | 10 ++++++- .../LinearAlgebra/test/structuredbroadcast.jl | 29 +++++++++++++++++-- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/stdlib/LinearAlgebra/src/structuredbroadcast.jl b/stdlib/LinearAlgebra/src/structuredbroadcast.jl index fcd5c68d48abe..68e0a163af664 100644 --- a/stdlib/LinearAlgebra/src/structuredbroadcast.jl +++ b/stdlib/LinearAlgebra/src/structuredbroadcast.jl @@ -102,6 +102,7 @@ function Base.similar(bc::Broadcasted{StructuredMatrixStyle{T}}, ::Type{ElType}) end function copyto!(dest::Diagonal, bc::Broadcasted{<:StructuredMatrixStyle}) + !isstructurepreserving(bc) && !fzeropreserving(bc) && copyto!(dest, convert(Broadcasted{Nothing}, bc)) axs = axes(dest) axes(bc) == axs || Broadcast.throwdm(axes(bc), axs) for i in axs[1] @@ -111,6 +112,7 @@ function copyto!(dest::Diagonal, bc::Broadcasted{<:StructuredMatrixStyle}) end function copyto!(dest::Bidiagonal, bc::Broadcasted{<:StructuredMatrixStyle}) + !isstructurepreserving(bc) && !fzeropreserving(bc) && copyto!(dest, convert(Broadcasted{Nothing}, bc)) axs = axes(dest) axes(bc) == axs || Broadcast.throwdm(axes(bc), axs) for i in axs[1] @@ -129,18 +131,22 @@ function copyto!(dest::Bidiagonal, bc::Broadcasted{<:StructuredMatrixStyle}) end function copyto!(dest::SymTridiagonal, bc::Broadcasted{<:StructuredMatrixStyle}) + !isstructurepreserving(bc) && !fzeropreserving(bc) && copyto!(dest, convert(Broadcasted{Nothing}, bc)) axs = axes(dest) axes(bc) == axs || Broadcast.throwdm(axes(bc), axs) for i in axs[1] dest.dv[i] = Broadcast._broadcast_getindex(bc, CartesianIndex(i, i)) end for i = 1:size(dest, 1)-1 - dest.ev[i] = Broadcast._broadcast_getindex(bc, CartesianIndex(i, i+1)) + v = Broadcast._broadcast_getindex(bc, CartesianIndex(i, i+1)) + v == Broadcast._broadcast_getindex(bc, CartesianIndex(i+1, i)) || throw(ArgumentError("broadcasted assignment breaks symmetry between locations ($i, $(i+1)) and ($(i+1), $i)")) + dest.ev[i] = v end return dest end function copyto!(dest::Tridiagonal, bc::Broadcasted{<:StructuredMatrixStyle}) + !isstructurepreserving(bc) && !fzeropreserving(bc) && copyto!(dest, convert(Broadcasted{Nothing}, bc)) axs = axes(dest) axes(bc) == axs || Broadcast.throwdm(axes(bc), axs) for i in axs[1] @@ -154,6 +160,7 @@ function copyto!(dest::Tridiagonal, bc::Broadcasted{<:StructuredMatrixStyle}) end function copyto!(dest::LowerTriangular, bc::Broadcasted{<:StructuredMatrixStyle}) + !isstructurepreserving(bc) && !fzeropreserving(bc) && copyto!(dest, convert(Broadcasted{Nothing}, bc)) axs = axes(dest) axes(bc) == axs || Broadcast.throwdm(axes(bc), axs) for j in axs[2] @@ -165,6 +172,7 @@ function copyto!(dest::LowerTriangular, bc::Broadcasted{<:StructuredMatrixStyle} end function copyto!(dest::UpperTriangular, bc::Broadcasted{<:StructuredMatrixStyle}) + !isstructurepreserving(bc) && !fzeropreserving(bc) && copyto!(dest, convert(Broadcasted{Nothing}, bc)) axs = axes(dest) axes(bc) == axs || Broadcast.throwdm(axes(bc), axs) for j in axs[2] diff --git a/stdlib/LinearAlgebra/test/structuredbroadcast.jl b/stdlib/LinearAlgebra/test/structuredbroadcast.jl index 2999da87094bb..c3aa70e2c964e 100644 --- a/stdlib/LinearAlgebra/test/structuredbroadcast.jl +++ b/stdlib/LinearAlgebra/test/structuredbroadcast.jl @@ -51,14 +51,37 @@ end A = rand(N, N) sA = A + copy(A') D = Diagonal(rand(N)) - B = Bidiagonal(rand(N), rand(N - 1), :U) + Bu = Bidiagonal(rand(N), rand(N - 1), :U) + Bl = Bidiagonal(rand(N), rand(N - 1), :L) T = Tridiagonal(rand(N - 1), rand(N), rand(N - 1)) + ◣ = LowerTriangular(rand(N,N)) + ◥ = UpperTriangular(rand(N,N)) + @test broadcast!(sin, copy(D), D) == Diagonal(sin.(D)) - @test broadcast!(sin, copy(B), B) == Bidiagonal(sin.(B), :U) + @test broadcast!(sin, copy(Bu), Bu) == Bidiagonal(sin.(Bu), :U) + @test broadcast!(sin, copy(Bl), Bl) == Bidiagonal(sin.(Bl), :L) @test broadcast!(sin, copy(T), T) == Tridiagonal(sin.(T)) + @test broadcast!(sin, copy(◣), ◣) == LowerTriangular(sin.(◣)) + @test broadcast!(sin, copy(◥), ◥) == UpperTriangular(sin.(◥)) @test broadcast!(*, copy(D), D, A) == Diagonal(broadcast(*, D, A)) - @test broadcast!(*, copy(B), B, A) == Bidiagonal(broadcast(*, B, A), :U) + @test broadcast!(*, copy(Bu), Bu, A) == Bidiagonal(broadcast(*, Bu, A), :U) + @test broadcast!(*, copy(Bl), Bl, A) == Bidiagonal(broadcast(*, Bl, A), :L) @test broadcast!(*, copy(T), T, A) == Tridiagonal(broadcast(*, T, A)) + @test broadcast!(*, copy(◣), ◣, A) == LowerTriangular(broadcast(*, ◣, A)) + @test broadcast!(*, copy(◥), ◥, A) == UpperTriangular(broadcast(*, ◥, A)) + + @test_throws ArgumentError broadcast!(cos, copy(D), D) == Diagonal(sin.(D)) + @test_throws ArgumentError broadcast!(cos, copy(Bu), Bu) == Bidiagonal(sin.(Bu), :U) + @test_throws ArgumentError broadcast!(cos, copy(Bl), Bl) == Bidiagonal(sin.(Bl), :L) + @test_throws ArgumentError broadcast!(cos, copy(T), T) == Tridiagonal(sin.(T)) + @test_throws ArgumentError broadcast!(cos, copy(◣), ◣) == LowerTriangular(sin.(◣)) + @test_throws ArgumentError broadcast!(cos, copy(◥), ◥) == UpperTriangular(sin.(◥)) + @test_throws ArgumentError broadcast!(+, copy(D), D, A) == Diagonal(broadcast(*, D, A)) + @test_throws ArgumentError broadcast!(+, copy(Bu), Bu, A) == Bidiagonal(broadcast(*, Bu, A), :U) + @test_throws ArgumentError broadcast!(+, copy(Bl), Bl, A) == Bidiagonal(broadcast(*, Bl, A), :L) + @test_throws ArgumentError broadcast!(+, copy(T), T, A) == Tridiagonal(broadcast(*, T, A)) + @test_throws ArgumentError broadcast!(+, copy(◣), ◣, A) == LowerTriangular(broadcast(*, ◣, A)) + @test_throws ArgumentError broadcast!(+, copy(◥), ◥, A) == UpperTriangular(broadcast(*, ◥, A)) end @testset "map[!] over combinations of structured matrices" begin From e464d98020ad4f5bede3aaff7ae90042b1e75f5f Mon Sep 17 00:00:00 2001 From: Matt Bauman Date: Wed, 10 Apr 2019 17:31:11 -0400 Subject: [PATCH 2/2] Actually return from the quick-exit check --- stdlib/LinearAlgebra/src/structuredbroadcast.jl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/stdlib/LinearAlgebra/src/structuredbroadcast.jl b/stdlib/LinearAlgebra/src/structuredbroadcast.jl index 68e0a163af664..d449e66da74bf 100644 --- a/stdlib/LinearAlgebra/src/structuredbroadcast.jl +++ b/stdlib/LinearAlgebra/src/structuredbroadcast.jl @@ -102,7 +102,7 @@ function Base.similar(bc::Broadcasted{StructuredMatrixStyle{T}}, ::Type{ElType}) end function copyto!(dest::Diagonal, bc::Broadcasted{<:StructuredMatrixStyle}) - !isstructurepreserving(bc) && !fzeropreserving(bc) && copyto!(dest, convert(Broadcasted{Nothing}, bc)) + !isstructurepreserving(bc) && !fzeropreserving(bc) && return copyto!(dest, convert(Broadcasted{Nothing}, bc)) axs = axes(dest) axes(bc) == axs || Broadcast.throwdm(axes(bc), axs) for i in axs[1] @@ -112,7 +112,7 @@ function copyto!(dest::Diagonal, bc::Broadcasted{<:StructuredMatrixStyle}) end function copyto!(dest::Bidiagonal, bc::Broadcasted{<:StructuredMatrixStyle}) - !isstructurepreserving(bc) && !fzeropreserving(bc) && copyto!(dest, convert(Broadcasted{Nothing}, bc)) + !isstructurepreserving(bc) && !fzeropreserving(bc) && return copyto!(dest, convert(Broadcasted{Nothing}, bc)) axs = axes(dest) axes(bc) == axs || Broadcast.throwdm(axes(bc), axs) for i in axs[1] @@ -131,7 +131,7 @@ function copyto!(dest::Bidiagonal, bc::Broadcasted{<:StructuredMatrixStyle}) end function copyto!(dest::SymTridiagonal, bc::Broadcasted{<:StructuredMatrixStyle}) - !isstructurepreserving(bc) && !fzeropreserving(bc) && copyto!(dest, convert(Broadcasted{Nothing}, bc)) + !isstructurepreserving(bc) && !fzeropreserving(bc) && return copyto!(dest, convert(Broadcasted{Nothing}, bc)) axs = axes(dest) axes(bc) == axs || Broadcast.throwdm(axes(bc), axs) for i in axs[1] @@ -146,7 +146,7 @@ function copyto!(dest::SymTridiagonal, bc::Broadcasted{<:StructuredMatrixStyle}) end function copyto!(dest::Tridiagonal, bc::Broadcasted{<:StructuredMatrixStyle}) - !isstructurepreserving(bc) && !fzeropreserving(bc) && copyto!(dest, convert(Broadcasted{Nothing}, bc)) + !isstructurepreserving(bc) && !fzeropreserving(bc) && return copyto!(dest, convert(Broadcasted{Nothing}, bc)) axs = axes(dest) axes(bc) == axs || Broadcast.throwdm(axes(bc), axs) for i in axs[1] @@ -160,7 +160,7 @@ function copyto!(dest::Tridiagonal, bc::Broadcasted{<:StructuredMatrixStyle}) end function copyto!(dest::LowerTriangular, bc::Broadcasted{<:StructuredMatrixStyle}) - !isstructurepreserving(bc) && !fzeropreserving(bc) && copyto!(dest, convert(Broadcasted{Nothing}, bc)) + !isstructurepreserving(bc) && !fzeropreserving(bc) && return copyto!(dest, convert(Broadcasted{Nothing}, bc)) axs = axes(dest) axes(bc) == axs || Broadcast.throwdm(axes(bc), axs) for j in axs[2] @@ -172,7 +172,7 @@ function copyto!(dest::LowerTriangular, bc::Broadcasted{<:StructuredMatrixStyle} end function copyto!(dest::UpperTriangular, bc::Broadcasted{<:StructuredMatrixStyle}) - !isstructurepreserving(bc) && !fzeropreserving(bc) && copyto!(dest, convert(Broadcasted{Nothing}, bc)) + !isstructurepreserving(bc) && !fzeropreserving(bc) && return copyto!(dest, convert(Broadcasted{Nothing}, bc)) axs = axes(dest) axes(bc) == axs || Broadcast.throwdm(axes(bc), axs) for j in axs[2]