Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Views of Flux OneHotArrays #1349

Closed
RS-Coop opened this issue Feb 1, 2022 · 4 comments
Closed

Views of Flux OneHotArrays #1349

RS-Coop opened this issue Feb 1, 2022 · 4 comments
Labels
enhancement New feature or request

Comments

@RS-Coop
Copy link

RS-Coop commented Feb 1, 2022

The behviour of views and related functionality on CUDA arrays behaves as expected, but causes problems on Flux OneHotArrays. Specifically, the indices of the views aren't stored in a CUDA array which causes scalar indexing issues and an isbits error. The following example gives some insight into the issue, although it doesn't show the errors that eventually crop up:

using Flux, CUDA

data = rand(1:10, 10, 4)

x = data |> gpu
xhot = Flux.onehotbatch(data, 1:10) |> gpu

typeof(view(x, :, [1,3]))
#=
SubArray{Int64, 2, CuArray{Int64, 2, CUDA.Mem.DeviceBuffer},
Tuple{Base.Slice{Base.OneTo{Int64}}, CuArray{Int64, 1, CUDA.Mem.DeviceBuffer}}, false}
=#

typeof(view(xhot, :, :, [1,3]))
#=
SubArray{Bool, 3, Flux.OneHotArray{UInt32, 10, 2, 3, CuArray{UInt32, 2, CUDA.Mem.DeviceBuffer}},
Tuple{Base.Slice{Base.OneTo{Int64}}, Base.Slice{Base.OneTo{Int64}}, Vector{Int64}}, false}
=#

This may be too niche of a problem, but it seems possible this could be an issue in other places where the structure isn't a CUDA array itself, but uses CUDA arrays. It would be nice if this could be automatically detected, and then view would put all indices on CUDA arrays.

The following code gets around the issue, but it doesn't seem ideal:

xv = view(x, :, [1,3])
typeof(Base.unsafe_view(xhot, Base.Slice(Base.OneTo(10)), Base.Slice(Base.OneTo(10)), xv.indices[ndims(x)]))
@RS-Coop RS-Coop added the enhancement New feature or request label Feb 1, 2022
@maleadt
Copy link
Member

maleadt commented Feb 8, 2022

Specifically, the indices of the views aren't stored in a CUDA array which causes scalar indexing issues and an isbits error.

CuArray is only used to represent contiguous or strided arrays, which isn't the case when you have a view with indices. The fact that some views are represented by a CuArray is already a stretch -- only done so because of load time issues when using SubArray{..., <:CuArray} everywhere -- and I don't think we should extend this to views with indices as these cannot be passed to e.g. CUBLAS or other NVIDIA libraries.

The fact that SubArray{..., Flux.OneHotArray{<:CuArray}} isn't a CuArray and doesn't have the specialized methods CUDA.jl provides for a singly-wrapped SubArray{..., <:CuArray} is something that should be fixed in Base, e.g. with JuliaLang/julia#31563 which would allow to identify the wrapped CuArray without a combinatorial explosion.

@maleadt maleadt closed this as completed Feb 8, 2022
@RS-Coop
Copy link
Author

RS-Coop commented Feb 8, 2022

Just so I have the correct understanding, can you answer the following two questions:

  • Is the idea of views of CuArrays fundamentally impractical because they are supposed to be contiguous?
  • Is it possible to use sparse CuArrays instead of Flux one hot arrays to achieve the same goal?

Thanks for the info

@maleadt
Copy link
Member

maleadt commented Feb 9, 2022

Is the idea of views of CuArrays fundamentally impractical because they are supposed to be contiguous?

No, views are just supported. Contiguous ones are represented by CuArray objects, all other views just use SubArray. Nothing impractical here.

Is it possible to use sparse CuArrays instead of Flux one hot arrays to achieve the same goal?

That depends on Flux, but I think there's going to be missing functionality. E.g. CuSparseArray doesn't support broadcast.

@RS-Coop
Copy link
Author

RS-Coop commented Feb 9, 2022

Okay, thanks for taking the time to answer my questions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants