OneHotArrays.jl icon indicating copy to clipboard operation
OneHotArrays.jl copied to clipboard

Ambiguity in `getindex`, and missing `==` definition?

Open mcabbott opened this issue 2 years ago • 4 comments

The following error showed up in #27:

julia> y1 = onehotbatch([1, 3, 0, 2], 0:9);

julia> y1 == y1
true

julia> using JLArrays

julia> y2 = onehotbatch([1, 3, 0, 2] |> jl, 0:9)  # with #27
10×4 OneHotMatrix(::JLArray{UInt32, 1}) with eltype Bool:
 ⋅  ⋅  1  ⋅
 1  ⋅  ⋅  ⋅
 ⋅  ⋅  ⋅  1
 ⋅  1  ⋅  ⋅
 ⋅  ⋅  ⋅  ⋅
 ⋅  ⋅  ⋅  ⋅
 ⋅  ⋅  ⋅  ⋅
 ⋅  ⋅  ⋅  ⋅
 ⋅  ⋅  ⋅  ⋅
 ⋅  ⋅  ⋅  ⋅

julia> jl(y1) == y2
ERROR: MethodError: getindex(::OneHotMatrix{UInt32, JLArray{UInt32, 1}}, ::Int64, ::Int64) is ambiguous.

Candidates:
  getindex(x::OneHotArray{<:Any, N, <:Any, <:GPUArraysCore.AbstractGPUArray}, i::Int64, I::Vararg{Any, N}) where N
    @ OneHotArrays ~/.julia/dev/OneHotArrays/src/array.jl:71
  getindex(x::OneHotArray{var"#s8", N, var"N+1", I} where {var"#s8", var"N+1", I<:Union{AbstractArray{var"#s8", N}, var"#s8"}}, i::Int64, I::Vararg{Int64, N}) where N
    @ OneHotArrays ~/.julia/dev/OneHotArrays/src/array.jl:65
To resolve the ambiguity, try making one of the methods more specific, or adding a new method more specific than any of the existing applicable methods.

Stacktrace:
 [1] _getindex
   @ ./abstractarray.jl:1344 [inlined]
 [2] getindex
   @ ./abstractarray.jl:1294 [inlined]
 [3] iterate
   @ ./abstractarray.jl:1220 [inlined]
 [4] iterate
   @ ./abstractarray.jl:1218 [inlined]
 [5] _zip_iterate_some
   @ ./iterators.jl:428 [inlined]
 [6] _zip_iterate_all
   @ ./iterators.jl:420 [inlined]
 [7] iterate
   @ ./iterators.jl:410 [inlined]
 [8] ==(A::OneHotMatrix{UInt32, JLArray{UInt32, 1}}, B::OneHotMatrix{UInt32, JLArray{UInt32, 1}})
   @ Base ./abstractarray.jl:2912
 [9] top-level scope
   @ REPL[34]:1

mcabbott avatar Dec 27 '22 17:12 mcabbott

I think these signatures need to be made more specific to include GPUArray + Int + Vararg{Int} as a combo.

darsnack avatar Dec 27 '22 20:12 darsnack

I haven't investigated but those do look a bit non-standard, the usual form is getindex(x::AbstractArray{<:Any, N}, I::Vararg{Int, N}) where N with N==ndims(x).

mcabbott avatar Dec 27 '22 21:12 mcabbott

This error also occurs when OneHotArrays is used with CUDA, not sure what the correct solution is

julia> a = onehotbatch([1,2,3],1:4) |> gpu
4×3 OneHotMatrix(::CuArray{UInt32, 1, CUDA.Mem.DeviceBuffer}) with eltype Bool:
 1  ⋅  ⋅
 ⋅  1  ⋅
 ⋅  ⋅  1
 ⋅  ⋅  ⋅

julia> eachslice(a;dims = 2)
3-element ColumnSlices{OneHotMatrix{UInt32, CuArray{UInt32, 1, CUDA.Mem.DeviceBuffer}}, Tuple{Base.OneTo{Int64}}, SubArray{Bool, 1, OneHotMatrix{UInt32, CuArray{UInt32, 1, CUDA.Mem.DeviceBuffer}}, Tuple{Base.Slice{Base.OneTo{Int64}}, Int64}, false}}:
Error showing value of type ColumnSlices{OneHotMatrix{UInt32, CuArray{UInt32, 1, CUDA.Mem.DeviceBuffer}}, Tuple{Base.OneTo{Int64}}, SubArray{Bool, 1, OneHotMatrix{UInt32, CuArray{UInt32, 1, CUDA.Mem.DeviceBuffer}}, Tuple{Base.Slice{Base.OneTo{Int64}}, Int64}, false}}:
ERROR: MethodError: getindex(::OneHotMatrix{UInt32, CuArray{UInt32, 1, CUDA.Mem.DeviceBuffer}}, ::Int64, ::Int64) is ambiguous.

Candidates:
  getindex(x::OneHotArray{var"#s3", N, var"N+1", I} where {var"#s3", var"N+1", I<:Union{AbstractArray{var"#s3", N}, var"#s3"}}, i::Int64, I::Vararg{Int64, N}) where N
    @ OneHotArrays ~/.julia/packages/OneHotArrays/n10pv/src/array.jl:65                                               
  getindex(x::OneHotArray{<:Any, N, <:Any, <:GPUArraysCore.AbstractGPUArray}, i::Int64, I::Vararg{Any, N}) where N
    @ OneHotArrays ~/.julia/packages/OneHotArrays/n10pv/src/array.jl:71                                               

Possible fix, define
  getindex(::OneHotArray{var"#s3", N, <:Any, <:Union{…}} where var"#s3", ::Int64, ::Vararg{Int64, N}) where N

Stacktrace:
  [1] isassigned(::OneHotMatrix{UInt32, CuArray{UInt32, 1, CUDA.Mem.DeviceBuffer}}, ::Int64, ::Int64)                 
    @ Base ./multidimensional.jl:1578
  [2] isassigned
    @ ./subarray.jl:362 [inlined]
  [3] show_delim_array(io::IOContext{…}, itr::SubArray{…}, op::Char, delim::String, cl::Char, delim_one::Bool, i1::Int64, l::Int64)
    @ Base ./show.jl:1342
  [4] show_delim_array
    @ ./show.jl:1335 [inlined]
  [5] show_vector(io::IOContext{IOBuffer}, v::SubArray{Bool, 1, OneHotMatrix{…}, Tuple{…}, false}, opn::Char, cls::Char)
    @ Base ./arrayshow.jl:530
  [6] show_vector
    @ ./arrayshow.jl:515 [inlined]
  [7] show(io::IOContext{IOBuffer}, X::SubArray{Bool, 1, OneHotMatrix{…}, Tuple{…}, false})                           
    @ Base ./arrayshow.jl:486
  [8] sprint(f::Function, args::SubArray{…}; context::IOContext{…}, sizehint::Int64)
    @ Base ./strings/io.jl:112
  [9] sprint
    @ ./strings/io.jl:107 [inlined]
 [10] alignment_from_show
    @ ./show.jl:3003 [inlined]
 [11] alignment(io::IOContext{Base.TTY}, x::SubArray{Bool, 1, OneHotMatrix{…}, Tuple{…}, false})                      
    @ Base ./show.jl:3022
 [12] _print_matrix(io::IOContext{…}, X::AbstractVecOrMat, pre::String, sep::String, post::String, hdots::String, vdots::String, ddots::String, hmod::Int64, vmod::Int64, rowsA::UnitRange{…}, colsA::UnitRange{…})
    @ Base ./arrayshow.jl:207
 [13] print_matrix(io::IOContext{…}, X::ColumnSlices{…}, pre::String, sep::String, post::String, hdots::String, vdots::String, ddots::String, hmod::Int64, vmod::Int64)
    @ Base ./arrayshow.jl:171
 [14] print_matrix
    @ ./arrayshow.jl:171 [inlined]
 [15] print_array
    @ ./arrayshow.jl:358 [inlined]
 [16] show(io::IOContext{Base.TTY}, ::MIME{Symbol("text/plain")}, X::ColumnSlices{OneHotMatrix{…}, Tuple{…}, SubArray{…}})                                                       
    @ Base ./arrayshow.jl:399
Some type information was truncated. Use `show(err)` to see complete types.

NeroBlackstone avatar May 21 '24 11:05 NeroBlackstone

A workaround is use Matrix to convert OneHotMatrix to Matrix:

julia> x = Matrix(onehotbatch([1,2], 1:3)) |> cu
3×2 CuArray{Bool, 2, CUDA.Mem.DeviceBuffer}:
 1  0
 0  1
 0  0

julia> eachslice(x;dims=2)
2-element ColumnSlices{CuArray{Bool, 2, CUDA.Mem.DeviceBuffer}, Tuple{Base.OneTo{Int64}}, CuArray{Bool, 1, CUDA.Mem.DeviceBuffer}}:
 Bool[1, 0, 0]
 Bool[0, 1, 0]

NeroBlackstone avatar May 21 '24 14:05 NeroBlackstone