search

rafaqz / DimensionalData.jl

Named dimensions and indexing for julia arrays and other data
https://rafaqz.github.io/DimensionalData.jl/stable/
MIT License
281 stars 41 forks source link

broadcast fails for 0-dimensional DimArray #421

Closed sethaxen closed 1 year ago

sethaxen commented 1 year ago

isapprox for 0-dimensional DimArray seems to try to use a method of rebuild that is not defined:

julia> using DimensionalData

julia> x = DimArray(fill(3), ())
0-dimensional DimArray{Int64,0}: 

3

julia> y = DimArray(fill(3), ())
0-dimensional DimArray{Int64,0}: 

3

julia> Array(x) ≈ Array(y)
true

julia> x ≈ y
ERROR: MethodError: no method matching rebuild(::DimArray{Int64, 0, Tuple{}, Tuple{}, Array{Int64, 0}, DimensionalData.NoName, DimensionalData.Dimensions.LookupArrays.NoMetadata}, ::Int64, ::Tuple{}, ::Tuple{}, ::Symbol, ::DimensionalData.Dimensions.LookupArrays.NoMetadata)
Closest candidates are:
  rebuild(::DimArray, ::AbstractArray, ::Tuple, ::Tuple, ::Any, ::Any) at ~/.julia/packages/DimensionalData/K9D4P/src/array/array.jl:272
  rebuild(::AbstractDimArray, ::Any, ::Tuple, ::Any, ::Any) at ~/.julia/packages/DimensionalData/K9D4P/src/array/array.jl:49
  rebuild(::AbstractDimStack, ::Any, ::Any, ::Any, ::Any, ::Any) at ~/.julia/packages/DimensionalData/K9D4P/src/stack/stack.jl:80
  ...
Stacktrace:
 [1] rebuild
   @ ~/.julia/packages/DimensionalData/K9D4P/src/array/array.jl:52 [inlined]
 [2] copy(bc::Base.Broadcast.Broadcasted{DimensionalData.DimensionalStyle{Base.Broadcast.DefaultArrayStyle{0}}, Tuple{}, typeof(-), Tuple{DimArray{Int64, 0, Tuple{}, Tuple{}, Array{Int64, 0}, DimensionalData.NoName, DimensionalData.Dimensions.LookupArrays.NoMetadata}, DimArray{Int64, 0, Tuple{}, Tuple{}, Array{Int64, 0}, DimensionalData.NoName, DimensionalData.Dimensions.LookupArrays.NoMetadata}}})
   @ DimensionalData ~/.julia/packages/DimensionalData/K9D4P/src/array/broadcast.jl:43
 [3] materialize
   @ ./broadcast.jl:860 [inlined]
 [4] broadcast_preserving_zero_d
   @ ./broadcast.jl:849 [inlined]
 [5] -(A::DimArray{Int64, 0, Tuple{}, Tuple{}, Array{Int64, 0}, DimensionalData.NoName, DimensionalData.Dimensions.LookupArrays.NoMetadata}, B::DimArray{Int64, 0, Tuple{}, Tuple{}, Array{Int64, 0}, DimensionalData.NoName, DimensionalData.Dimensions.LookupArrays.NoMetadata})
   @ Base ./arraymath.jl:8
 [6] isapprox(x::DimArray{Int64, 0, Tuple{}, Tuple{}, Array{Int64, 0}, DimensionalData.NoName, DimensionalData.Dimensions.LookupArrays.NoMetadata}, y::DimArray{Int64, 0, Tuple{}, Tuple{}, Array{Int64, 0}, DimensionalData.NoName, DimensionalData.Dimensions.LookupArrays.NoMetadata}; atol::Int64, rtol::Int64, nans::Bool, norm::typeof(LinearAlgebra.norm))
   @ LinearAlgebra ~/.julia/juliaup/julia-1.8.2+0.x64/share/julia/stdlib/v1.8/LinearAlgebra/src/generic.jl:1700
 [7] isapprox(x::DimArray{Int64, 0, Tuple{}, Tuple{}, Array{Int64, 0}, DimensionalData.NoName, DimensionalData.Dimensions.LookupArrays.NoMetadata}, y::DimArray{Int64, 0, Tuple{}, Tuple{}, Array{Int64, 0}, DimensionalData.NoName, DimensionalData.Dimensions.LookupArrays.NoMetadata})
   @ LinearAlgebra ~/.julia/juliaup/julia-1.8.2+0.x64/share/julia/stdlib/v1.8/LinearAlgebra/src/generic.jl:1696
 [8] top-level scope
   @ REPL[17]:1
rafaqz commented 1 year ago

Looks like there is a bug in zero dimensional broadcast.

Base.Broadcast.materialise unwraps the zero dim array and just returns a scalar.

We have no special casing like that for zero dims here so we try to rebuild with that unwrapped scalar and hit this error.

Line 43 of broadcast.jl needs to add a check that ndims(A) == 0 then just return data as is.

sethaxen commented 1 year ago

Okay, would you like me to open a PR with the proposed fix?

rafaqz commented 1 year ago

Always good to get PRs, I guess your example can be the test.

sethaxen commented 1 year ago

broadcast_dims similarly fails for 0-dimensional DimArrays, which I'll fix in the same PR:

julia> x = DimArray(fill(3), ())
0-dimensional DimArray{Int64,0}: 

3

julia> broadcast_dims(+, x, x)
ERROR: MethodError: no method matching broadcast_dims!(::typeof(+), ::Array{Int64, 0}, ::DimArray{Int64, 0, Tuple{}, Tuple{}, Array{Int64, 0}, DimensionalData.NoName, DimensionalData.Dimensions.LookupArrays.NoMetadata}, ::DimArray{Int64, 0, Tuple{}, Tuple{}, Array{Int64, 0}, DimensionalData.NoName, DimensionalData.Dimensions.LookupArrays.NoMetadata})
Closest candidates are:
  broadcast_dims!(::Any, ::AbstractDimArray{<:Any, N}, ::AbstractDimArray...) where N at ~/.julia/packages/DimensionalData/K9D4P/src/utils.jl:113
Stacktrace:
 [1] broadcast_dims(::Function, ::DimArray{Int64, 0, Tuple{}, Tuple{}, Array{Int64, 0}, DimensionalData.NoName, DimensionalData.Dimensions.LookupArrays.NoMetadata}, ::Vararg{DimArray{Int64, 0, Tuple{}, Tuple{}, Array{Int64, 0}, DimensionalData.NoName, DimensionalData.Dimensions.LookupArrays.NoMetadata}})
   @ DimensionalData ~/.julia/packages/DimensionalData/K9D4P/src/utils.jl:96
 [2] top-level scope
   @ REPL[4]:1