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

Fusing Wrappers #467

Closed
jonas-eschmann opened this issue Oct 4, 2020 · 3 comments
Closed

Fusing Wrappers #467

jonas-eschmann opened this issue Oct 4, 2020 · 3 comments
Labels
enhancement New feature or request

Comments

@jonas-eschmann
Copy link

I would like to apply a Flux model on a large batch of a reshaped view of a CuArray. Using the view or reshape operations in isolation works, but it breaks down when chaining them:
Code with output

using CUDA
julia> CUDA.allowscalar(false)
julia> CUDA.ones(Float32, 10, 10) * view(CUDA.ones(Float32, 11, 10), 1:10, :)
10×10 CuArray{Float32,2}:
...
julia> CUDA.ones(Float32, 10, 10) * reshape(CUDA.ones(Float32, 100), 10, 10)
10×10 CuArray{Float32,2}:
...
julia> CUDA.ones(Float32, 10, 10) * reshape(view(CUDA.ones(Float32, 11, 5, 2), 1:10, :, :), 10, 10)
ERROR: scalar getindex is disallowed
...

Code to copy & paste

using CUDA
CUDA.allowscalar(false)
CUDA.ones(Float32, 10, 10) * view(CUDA.ones(Float32, 11, 10), 1:10, :)
CUDA.ones(Float32, 10, 10) * reshape(CUDA.ones(Float32, 100), 10, 10)
CUDA.ones(Float32, 10, 10) * reshape(view(CUDA.ones(Float32, 11, 5, 2), 1:10, :, :), 10, 10)

Actually CUDA.ones(Float32, 10, 10) * view(CUDA.ones(Float32, 11, 10), 1:10, :) does not seem to work in 1.3.3 but works in 2.0.0 so I was wondering if the chaining of these wrappers is also on the roadmap.

@jonas-eschmann jonas-eschmann added the enhancement New feature or request label Oct 4, 2020
@jonas-eschmann
Copy link
Author

It also seems that taking a view of a reshaped CuArray works (in 2.0.0)

CUDA.ones(Float32, 10, 10) * view(reshape(CUDA.ones(Float32, 11, 5, 2), 11, 10), 1:10, :)

@maleadt
Copy link
Member

maleadt commented Oct 5, 2020

Since we now just re-use Base's wrappers (SubArray, ReinterpretArray, ReshapeArray), this would better suited as a feature request there. It also applies there, where flattening wrappers would allow for more use of CPU BLAS.

Note that view of reshape works in 2.0 because of this type definitions:

CUDA.jl/src/array.jl

Lines 149 to 155 in b83fc1b

DenseReinterpretCuArray{T,N,A<:Union{CuArray,ContiguousSubCuArray}} = Base.ReinterpretArray{T,N,S,A} where S
DenseReshapedCuArray{T,N,A<:Union{CuArray,ContiguousSubCuArray,DenseReinterpretCuArray}} = Base.ReshapedArray{T,N,A}
DenseSubCuArray{T,N,A<:Union{CuArray,DenseReshapedCuArray,DenseReinterpretCuArray}} = Base.FastContiguousSubArray{T,N,A}
DenseCuArray{T,N} = Union{CuArray{T,N}, DenseSubCuArray{T,N}, DenseReshapedCuArray{T,N}, DenseReinterpretCuArray{T,N}}
DenseCuVector{T} = DenseCuArray{T,1}
DenseCuMatrix{T} = DenseCuArray{T,2}
DenseCuVecOrMat{T} = Union{DenseCuVector{T}, DenseCuMatrix{T}}

We could extend these for other nested wrappers, but the added complexity then regresses load time. So it would be better for wrapper constructors to perform fusion/flattening. In anticipation of such a change in Base, I'd be happy to put some specialized constructors in CUDA.jl though, so feel free to propose a PR.

@maleadt
Copy link
Member

maleadt commented Oct 20, 2020

view/reshape/reinterpret now result in a CuArray again.

@maleadt maleadt closed this as completed Oct 20, 2020
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