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

similar(reinterpret(T, view(::CuArray, ...))) produces Array instead of CuArray #602

Closed
simeonschaub opened this issue Dec 21, 2020 · 5 comments
Labels
bug Something isn't working

Comments

@simeonschaub
Copy link

It seems like this falls back to the generic implementation for AbstractArrays in Base, which always produces an Array:

julia> similar(reinterpret(SVector{4,Float32}, view(cu(rand(Float32, 10, 2)), 1:4, :)))
1×2 Array{SArray{Tuple{4},Float32,1,4},2}:
 [2.86328f16, 4.5768f-41, 0.0, 0.0]  [2.93855f16, 4.5768f-41, 2.86328f16, 4.5768f-41]

julia> similar(reinterpret(SVector{4,Float32}, cu(rand(Float32, 4, 2))))
1×2 CuArray{SArray{Tuple{4},Float32,1,4},2}:
 [0.683944, 0.111384, 0.644627, 0.645665]  [0.449726, 0.553998, 0.548373, 0.354193]

I think returning a CuArray would make more sense here, so generic code can still run on the GPU.

This is using CUDA 2.3.0 on Julia 1.5.3

@simeonschaub simeonschaub added the bug Something isn't working label Dec 21, 2020
@simeonschaub
Copy link
Author

Alternatively, maybe this should be fixed upstream, so similar(reinterpret(T, ::A), ...) falls back to similar(::A, T, ...)?

simeonschaub added a commit to JuliaLang/julia that referenced this issue Dec 22, 2020
We already do this for `SubArray` and `PermutedDimsArray`, so I think this makes sense here.

ref JuliaGPU/CUDA.jl#602
simeonschaub added a commit to JuliaLang/julia that referenced this issue Dec 22, 2020
We already do this for `SubArray`, `PermutedDimsArray`, and `ReshapedArray`, so I think this makes sense here. This also adds tests for `similar(::ReshapedArray)`, since I didn't notice that was actually defined before.

ref JuliaGPU/CUDA.jl#602
Keno pushed a commit to JuliaLang/julia that referenced this issue Dec 25, 2020
We already do this for `SubArray`, `PermutedDimsArray`, and `ReshapedArray`, so I think this makes sense here. This also adds tests for `similar(::ReshapedArray)`, since I didn't notice that was actually defined before.

ref JuliaGPU/CUDA.jl#602
@simeonschaub
Copy link
Author

This should be fixed on Julia nightly, but would it be a good idea to try to fix this for CuArrays for older versions as well?

@maleadt
Copy link
Member

maleadt commented Jan 5, 2021

We don't use ReinterpretArray anymore after #498, could you verify this works on master as expected?

@maleadt
Copy link
Member

maleadt commented Jan 5, 2021

So this works with contiguous reinterpred views

julia> similar(reinterpret(Int32, view(CUDA.rand(2,2), :, 1)))
2-element CuArray{Int32, 1}:
 0
 0

julia> similar(reinterpret(Int32, view(CUDA.rand(2,2), 1, :)))
2-element Vector{Int32}:
 15
  0

@simeonschaub
Copy link
Author

Awesome thanks! Works for me now.

ElOceanografo pushed a commit to ElOceanografo/julia that referenced this issue May 4, 2021
We already do this for `SubArray`, `PermutedDimsArray`, and `ReshapedArray`, so I think this makes sense here. This also adds tests for `similar(::ReshapedArray)`, since I didn't notice that was actually defined before.

ref JuliaGPU/CUDA.jl#602
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants