Empty array input will throw error

[yuehhua]

Closes #411

[YichengDWu]

I would argue that we should keep the feature. I'm using GNNs and have something like

    function message(xi, xj, e)
        return ϕ(cat(xi.f, xj.f, xi.u - xj.u, xi.x - xj.x, xi.θ, dims = 1))
    end

For different datasets, one of the features x and θ or both of them might be missing. It would be a mess if i have to explicitly handle those cases inside message

[yuehhua]

Buildkite fails on precompiling NNlibCUDA with following error:

ERROR: LoadError: LoadError: UndefVarError: upsample_linear_wcn! not defined

which is not relevant to this PR.

[yuehhua]

The CPU case is a coincidence

It is a coincidence we should avoid.

@MilkshakeForReal You just mentioned that this "coincidence" should be avoid in #411 . To achieve that, throwing an error is reasonable. Isn't it?

So, if your feature x is missing, what is the type of x?

[yuehhua]

Still some numerical error occur in Convolution AutoDiff: spatial_rank=3 on ubuntu, Julia 1.

[YichengDWu]

Sorry I was not clear enough. The coincidence I meant was NNlib.gather! supports CUDA arrays, but shouldn't we always use NNlibCUDA.gather! when dealing with CUDA arrays? I don't have any problem with NNlib.gather! itself.

So, if your feature x is missing, what is the type of x?

It is an array of size 0*num_samples. Julia's native syntax supports many operations on empty arrays

julia> a = rand(0,32)
0×32 Matrix{Float64}

julia> b = rand(3,32)
3×32 Matrix{Float64}:
 0.406172  0.986911  0.511794  0.0439804  0.0694194  0.448747   …  0.432278  0.304936  0.384311  0.22973    0.342039
 0.280945  0.327012  0.723847  0.0307628  0.422654   0.0926303     0.886039  0.132593  0.828088  0.29174    0.390778
 0.747481  0.384199  0.271736  0.809259   0.560512   0.914154      0.252876  0.879381  0.339615  0.0820635  0.429745

julia> vcat(a,b)
3×32 Matrix{Float64}:
 0.406172  0.986911  0.511794  0.0439804  0.0694194  0.448747   …  0.432278  0.304936  0.384311  0.22973    0.342039
 0.280945  0.327012  0.723847  0.0307628  0.422654   0.0926303     0.886039  0.132593  0.828088  0.29174    0.390778
 0.747481  0.384199  0.271736  0.809259   0.560512   0.914154      0.252876  0.879381  0.339615  0.0820635  0.429745

julia> a[:,3]
Float64[]

I think it's nice to inherit them.

[yuehhua]

@MilkshakeForReal So, what behavior do you expect that gather accept an argument with empty array?
Just return it directly? Or you want to have the size changed according to your index?

[YichengDWu]

It should behave excatly like Julia's native syntax

julia> a = rand(0,32)
0×32 Matrix{Float64}

julia> idx = [2,3,4]
3-element Vector{Int64}:
 2
 3
 4

julia> a[:,idx]
0×3 Matrix{Float64}

julia> a[:,[33]]
ERROR: BoundsError: attempt to access 0×32 Matrix{Float64} at index [1:0, [33]]
Stacktrace:
 [1] throw_boundserror(A::Matrix{Float64}, I::Tuple{Base.Slice{Base.OneTo{Int64}}, Vector{Int64}})
   @ Base .\abstractarray.jl:691
 [2] checkbounds
   @ .\abstractarray.jl:656 [inlined]
 [3] _getindex
   @ .\multidimensional.jl:838 [inlined]
 [4] getindex(::Matrix{Float64}, ::Function, ::Vector{Int64})
   @ Base .\abstractarray.jl:1218
 [5] top-level scope
   @ REPL[10]:1

Or NNlibCUDA.gather! should be like the current NNlib.gather!. The size changes according to the index.

[YichengDWu]

I have a crappy version of NNlibCUDA.gather!

function gather_checkbounds(X::AbstractArray{Tx,Nx},
    idx::AbstractArray,
    dims::Int) where
    {Tx,Nx}
    idx_ranges = axes(X)[dims+1,end]
    for i in idx
        Base.checkbounds_indices(Bool, idx_ranges, i) || throw_boundserror(X, i)
    end  
end

function NNlib.gather!(dst::AnyCuArray, src::AnyCuArray, idx::AnyCuArray)
    dims = gather_check_dims(src, dst, idx)
    gather_checkbounds(src, idx, dims)
    isempty(src) && return dst
    dims_size = size(src)[1:dims]
    max_dims_idx = prod(dims_size)
    max_idx = max_dims_idx * length(idx)
    args = dst, src, idx, max_idx, max_dims_idx, dims_size

    kernel = @cuda launch=false gather_kernel!(args...)
    config = launch_configuration(kernel.fun; max_threads=256)
    threads = min(max_idx, config.threads)
    blocks = cld(max_idx, threads)
    kernel(args...; threads=threads, blocks=blocks)
    return dst
end

[yuehhua]

For the bound check issue, it should be resolved in FluxML/NNlibCUDA.jl#51.

[ToucheSir]

Still some numerical error occur in Convolution AutoDiff: spatial_rank=3 on ubuntu, Julia 1.

That's #359, I still have no idea why it happens, and you can safely ignore it.

[yuehhua]

@ToucheSir Thank you for the hint.

[yuehhua]

julia> using NNlib, NNlibCUDA

julia> using CUDA

julia> a = rand(0,32) |> cu
0×32 CuArray{Float32, 2, CUDA.Mem.DeviceBuffer}

julia> idx = [2,3,4] |> cu
3-element CuArray{Int64, 1, CUDA.Mem.DeviceBuffer}:
 2
 3
 4

julia> NNlib.gather(a, idx)
ERROR: DivideError: integer division error
Stacktrace:
 [1] div
   @ ./int.jl:284 [inlined]
 [2] div
   @ ./div.jl:257 [inlined]
 [3] div
   @ ./div.jl:312 [inlined]
 [4] cld
   @ ./div.jl:269 [inlined]
 [5] gather!(dst::CuArray{Float32, 2, CUDA.Mem.DeviceBuffer}, src::CuArray{Float32, 2, CUDA.Mem.DeviceBuffer}, idx::CuArray{Int64, 1, CUDA.Mem.DeviceBuffer})
   @ NNlibCUDA /media/yuehhua/Workbench/workspace/NNlibCUDA.jl/src/gather.jl:81
 [6] gather(src::CuArray{Float32, 2, CUDA.Mem.DeviceBuffer}, idx::CuArray{Int64, 1, CUDA.Mem.DeviceBuffer})
   @ NNlib ~/.julia/packages/NNlib/hydo3/src/gather.jl:77
 [7] top-level scope
   @ REPL[43]:1
 [8] top-level scope
   @ ~/.julia/packages/CUDA/GGwVa/src/initialization.jl:52

It seems that gather works as expected on empty cpu array but not on empty gpu array.
We should have some work on empty gpu array.

[YichengDWu]

Adding one line here

chk = checkbounds_src(src, dims, eltype(idx))
isempty(src) && return dst

would do it

[yuehhua]

Support empty source array for gather in FluxML/NNlibCUDA.jl#51.