From a73a3d2aaf58fa902c557d682d5c522bb66cdcaa Mon Sep 17 00:00:00 2001 From: "Tamas K. Papp" Date: Sun, 21 Mar 2021 11:39:18 +0100 Subject: [PATCH 1/5] Normalize indices in promote_shape error messages. Seeing implementation like `Base.OneTo` in error messages may be confusing to some users (cf discussion in #39242, [discourse](https://discourse.julialang.org/t/promote-shape-dimension-mismatch/57529/)). This PR turns ```julia julia> ones(2, 3) + ones(3, 2) ERROR: DimensionMismatch("dimensions must match: a has dims (Base.OneTo(2), Base.OneTo(3)), b has dims (Base.OneTo(3), Base.OneTo(2)), mismatch at 1") ``` into ```julia julia> ones(2, 3) + ones(3, 2) ERROR: DimensionMismatch("dimensions must match: a has axes (1:2, 1:3), b has axes (1:3, 1:2), mismatch at 1") ``` Fixes #40118. Acked-by: Tamas K. Papp --- base/indices.jl | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/base/indices.jl b/base/indices.jl index ee3d076c5d7ec..3159d1e09a2c1 100644 --- a/base/indices.jl +++ b/base/indices.jl @@ -106,26 +106,34 @@ IndexStyle(::IndexStyle, ::IndexStyle) = IndexCartesian() promote_shape(::Tuple{}, ::Tuple{}) = () -function promote_shape(a::Tuple{Int,}, b::Tuple{Int,}) - if a[1] != b[1] - throw(DimensionMismatch("dimensions must match: a has dims $a, b has dims $b")) +# Consistent error message for promote_shape mismatch, hiding implementation details like +# OneTo. When b ≡ nothing, it is omitted; i can be supplied for an index. +function throw_promote_shape_mismatch(a, b, i = nothing) + _normalize(d) = map(x -> x isa AbstractUnitRange ? (firstindex(x):lastindex(x)) : x, d) + msg = "dimensions must match: a has dims $(_normalize(a))" + if b ≢ nothing + msg *= ", b has dims $(_normalize(b))" + end + if i ≢ nothing + msg *= ", mismatch at $(i)" end + throw(DimensionMismatch(msg)) +end + +function promote_shape(a::Tuple{Int,}, b::Tuple{Int,}) + a[1] != b[1] && throw_promote_shape_mismatch(a, b) return a end function promote_shape(a::Tuple{Int,Int}, b::Tuple{Int,}) - if a[1] != b[1] || a[2] != 1 - throw(DimensionMismatch("dimensions must match: a has dims $a, b has dims $b")) - end + (a[1] != b[1] || a[2] != 1) && throw_promote_shape_mismatch(a, b) return a end promote_shape(a::Tuple{Int,}, b::Tuple{Int,Int}) = promote_shape(b, a) function promote_shape(a::Tuple{Int, Int}, b::Tuple{Int, Int}) - if a[1] != b[1] || a[2] != b[2] - throw(DimensionMismatch("dimensions must match: a has dims $a, b has dims $b")) - end + (a[1] != b[1] || a[2] != b[2]) && throw_promote_shape_mismatch(a, b) return a end @@ -153,14 +161,10 @@ function promote_shape(a::Dims, b::Dims) return promote_shape(b, a) end for i=1:length(b) - if a[i] != b[i] - throw(DimensionMismatch("dimensions must match: a has dims $a, b has dims $b, mismatch at $i")) - end + a[i] != b[i] && throw_promote_shape_mismatch(a, b, i) end for i=length(b)+1:length(a) - if a[i] != 1 - throw(DimensionMismatch("dimensions must match: a has dims $a, must have singleton at dim $i")) - end + a[i] != 1 && throw_promote_shape_mismatch(a, nothing, i) end return a end @@ -174,14 +178,10 @@ function promote_shape(a::Indices, b::Indices) return promote_shape(b, a) end for i=1:length(b) - if a[i] != b[i] - throw(DimensionMismatch("dimensions must match: a has dims $a, b has dims $b, mismatch at $i")) - end + a[i] != b[i] && throw_promote_shape_mismatch(a, b, i) end for i=length(b)+1:length(a) - if a[i] != 1:1 - throw(DimensionMismatch("dimensions must match: a has dims $a, must have singleton at dim $i")) - end + a[i] != 1:1 && throw_promote_shape_mismatch(a, nothing, i) end return a end From 2060101412003904d68c383ccb384ebb7fc78836 Mon Sep 17 00:00:00 2001 From: "Tamas K. Papp" Date: Mon, 22 Mar 2021 15:33:31 +0100 Subject: [PATCH 2/5] customize error message based on types --- base/indices.jl | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/base/indices.jl b/base/indices.jl index 3159d1e09a2c1..2677cedebe627 100644 --- a/base/indices.jl +++ b/base/indices.jl @@ -108,11 +108,15 @@ promote_shape(::Tuple{}, ::Tuple{}) = () # Consistent error message for promote_shape mismatch, hiding implementation details like # OneTo. When b ≡ nothing, it is omitted; i can be supplied for an index. -function throw_promote_shape_mismatch(a, b, i = nothing) - _normalize(d) = map(x -> x isa AbstractUnitRange ? (firstindex(x):lastindex(x)) : x, d) - msg = "dimensions must match: a has dims $(_normalize(a))" +function throw_promote_shape_mismatch(a::Tuple{Vararg{T}}, + b::Union{Nothing,Tuple{Vararg{T}}}, + i = nothing) where {T} + _has_axes = T <: AbstractUnitRange + _normalize(d) = map(x -> _has_axes ? (firstindex(x):lastindex(x)) : x, d) + _things = _has_axes ? "axes" : "size" + msg = "dimensions must match: a has $(_things) $(_normalize(a))" if b ≢ nothing - msg *= ", b has dims $(_normalize(b))" + msg *= ", b has $(_things) $(_normalize(b))" end if i ≢ nothing msg *= ", mismatch at $(i)" From d20425cf195406f892d35f0c82dc9802cf8a2b02 Mon Sep 17 00:00:00 2001 From: "Tamas K. Papp" Date: Thu, 3 Jun 2021 13:56:21 +0200 Subject: [PATCH 3/5] fix ambiguity --- base/indices.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/base/indices.jl b/base/indices.jl index 2677cedebe627..f5f6b9611bbcb 100644 --- a/base/indices.jl +++ b/base/indices.jl @@ -108,8 +108,8 @@ promote_shape(::Tuple{}, ::Tuple{}) = () # Consistent error message for promote_shape mismatch, hiding implementation details like # OneTo. When b ≡ nothing, it is omitted; i can be supplied for an index. -function throw_promote_shape_mismatch(a::Tuple{Vararg{T}}, - b::Union{Nothing,Tuple{Vararg{T}}}, +function throw_promote_shape_mismatch(a::Tuple{T,Vararg{T}}, + b::Union{Nothing,Tuple{T,Vararg{T}}}, i = nothing) where {T} _has_axes = T <: AbstractUnitRange _normalize(d) = map(x -> _has_axes ? (firstindex(x):lastindex(x)) : x, d) From 74a626c16c4433efe137fb812a1d0bc0a58675cf Mon Sep 17 00:00:00 2001 From: "Tamas K. Papp" Date: Wed, 23 Jun 2021 13:07:33 +0200 Subject: [PATCH 4/5] remove redundant "dimensions must match" thanks @mcabbott --- base/indices.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/indices.jl b/base/indices.jl index f5f6b9611bbcb..5a77192d0391b 100644 --- a/base/indices.jl +++ b/base/indices.jl @@ -114,7 +114,7 @@ function throw_promote_shape_mismatch(a::Tuple{T,Vararg{T}}, _has_axes = T <: AbstractUnitRange _normalize(d) = map(x -> _has_axes ? (firstindex(x):lastindex(x)) : x, d) _things = _has_axes ? "axes" : "size" - msg = "dimensions must match: a has $(_things) $(_normalize(a))" + msg = "a has $(_things) $(_normalize(a))" if b ≢ nothing msg *= ", b has $(_things) $(_normalize(b))" end From ce7a16e3d4c64dd5aab0e6694811fcf8d5e53b2d Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Sat, 3 Feb 2024 05:46:55 +0000 Subject: [PATCH 5/5] some more improvements - avoid assuming the tuples have the same eltype - coerce the inputs to simpler representations before printing, if unambiguous - update a couple more error messages with the same text - use string builder, instead of repeated concatenation --- base/indices.jl | 33 ++++++++++++------- stdlib/LinearAlgebra/src/matmul.jl | 4 +-- .../LinearAlgebra/src/structuredbroadcast.jl | 4 ++- .../LinearAlgebra/test/structuredbroadcast.jl | 5 +++ 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/base/indices.jl b/base/indices.jl index 5a77192d0391b..9f2115e692564 100644 --- a/base/indices.jl +++ b/base/indices.jl @@ -106,22 +106,33 @@ IndexStyle(::IndexStyle, ::IndexStyle) = IndexCartesian() promote_shape(::Tuple{}, ::Tuple{}) = () -# Consistent error message for promote_shape mismatch, hiding implementation details like +# Consistent error message for promote_shape mismatch, hiding type details like # OneTo. When b ≡ nothing, it is omitted; i can be supplied for an index. -function throw_promote_shape_mismatch(a::Tuple{T,Vararg{T}}, - b::Union{Nothing,Tuple{T,Vararg{T}}}, - i = nothing) where {T} - _has_axes = T <: AbstractUnitRange - _normalize(d) = map(x -> _has_axes ? (firstindex(x):lastindex(x)) : x, d) - _things = _has_axes ? "axes" : "size" - msg = "a has $(_things) $(_normalize(a))" +function throw_promote_shape_mismatch(a::Tuple, b::Union{Nothing,Tuple}, i = nothing) + if a isa Tuple{Vararg{Base.OneTo}} && (b === nothing || b isa Tuple{Vararg{Base.OneTo}}) + a = map(lastindex, a)::Dims + b === nothing || (b = map(lastindex, b)::Dims) + end + _has_axes = !(a isa Dims && (b === nothing || b isa Dims)) + if _has_axes + _normalize(d) = map(x -> firstindex(x):lastindex(x), d) + a = _normalize(a) + b === nothing || (b = _normalize(b)) + _things = "axes " + else + _things = "size " + end + msg = IOBuffer() + print(msg, "a has ", _things) + print(msg, a) if b ≢ nothing - msg *= ", b has $(_things) $(_normalize(b))" + print(msg, ", b has ", _things) + print(msg, b) end if i ≢ nothing - msg *= ", mismatch at $(i)" + print(msg, ", mismatch at dim ", i) end - throw(DimensionMismatch(msg)) + throw(DimensionMismatch(String(take!(msg)))) end function promote_shape(a::Tuple{Int,}, b::Tuple{Int,}) diff --git a/stdlib/LinearAlgebra/src/matmul.jl b/stdlib/LinearAlgebra/src/matmul.jl index 9f163f8d394b4..c84a85ebc7c81 100644 --- a/stdlib/LinearAlgebra/src/matmul.jl +++ b/stdlib/LinearAlgebra/src/matmul.jl @@ -184,7 +184,7 @@ function Base.muladd(A::AbstractMatrix, y::AbstractVecOrMat, z::Union{Number, Ab end for d in ndims(Ay)+1:ndims(z) # Similar error to what Ay + z would give, to match (Any,Any,Any) method: - size(z,d) > 1 && throw(DimensionMismatch(string("dimensions must match: z has dims ", + size(z,d) > 1 && throw(DimensionMismatch(string("z has dims ", axes(z), ", must have singleton at dim ", d))) end Ay .+ z @@ -197,7 +197,7 @@ function Base.muladd(u::AbstractVector, v::AdjOrTransAbsVec, z::Union{Number, Ab end for d in 3:ndims(z) # Similar error to (u*v) + z: - size(z,d) > 1 && throw(DimensionMismatch(string("dimensions must match: z has dims ", + size(z,d) > 1 && throw(DimensionMismatch(string("z has dims ", axes(z), ", must have singleton at dim ", d))) end (u .* v) .+ z diff --git a/stdlib/LinearAlgebra/src/structuredbroadcast.jl b/stdlib/LinearAlgebra/src/structuredbroadcast.jl index 02e39b199679b..a227b678c9934 100644 --- a/stdlib/LinearAlgebra/src/structuredbroadcast.jl +++ b/stdlib/LinearAlgebra/src/structuredbroadcast.jl @@ -251,6 +251,8 @@ end # We can also implement `map` and its promotion in terms of broadcast with a stricter dimension check function map(f, A::StructuredMatrix, Bs::StructuredMatrix...) sz = size(A) - all(map(B->size(B)==sz, Bs)) || throw(DimensionMismatch("dimensions must match")) + for B in Bs + size(B) == sz || Base.throw_promote_shape_mismatch(sz, size(B)) + end return f.(A, Bs...) end diff --git a/stdlib/LinearAlgebra/test/structuredbroadcast.jl b/stdlib/LinearAlgebra/test/structuredbroadcast.jl index 2ca1904b2ff2d..fab6cd80141b2 100644 --- a/stdlib/LinearAlgebra/test/structuredbroadcast.jl +++ b/stdlib/LinearAlgebra/test/structuredbroadcast.jl @@ -142,6 +142,11 @@ end @test map!(*, Z, X, Y) == broadcast(*, fX, fY) end end + # these would be valid for broadcast, but not for map + @test_throws DimensionMismatch map(+, D, Diagonal(rand(1))) + @test_throws DimensionMismatch map(+, D, Diagonal(rand(1)), D) + @test_throws DimensionMismatch map(+, D, D, Diagonal(rand(1))) + @test_throws DimensionMismatch map(+, Diagonal(rand(1)), D, D) end @testset "Issue #33397" begin