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

Missing dispatch for indexing of reshaped arrays #556

Closed
ChrisRackauckas opened this issue Nov 21, 2020 · 7 comments
Closed

Missing dispatch for indexing of reshaped arrays #556

ChrisRackauckas opened this issue Nov 21, 2020 · 7 comments
Labels
bug Something isn't working

Comments

@ChrisRackauckas
Copy link
Member

x = cu(rand(8))
W = @view p[reshape(1:4,2,2)]
W' # fails

This is because W is a SubArray`. However, the other method works:

x = cu(rand(8))
W = reshape(@view(p[1:4]),2,2)
W'

This is different from Array semantics:

x = rand(8)
W = reshape(@view(x[1:4]),2,2)
W'

x = rand(8)
W = @view x[reshape(1:4,2,2)]
W'

where the latter is more optimized with BLAS due to some of @mbauman 's fanciness.

@ChrisRackauckas ChrisRackauckas added the bug Something isn't working label Nov 21, 2020
@maleadt
Copy link
Member

maleadt commented Dec 1, 2020

What do you expect here exactly? @view x[reshape(1:4,2,2)] is a non-contiguous SubArray, both on the CPU and the GPU, because of the indices. Calling transpose on that falls back to the generic implementation on the CPU, and can't possibly work on the GPU because of JuliaGPU/Adapt.jl#21 once again.

@ChrisRackauckas
Copy link
Member Author

It's contiguous:

julia> x = reshape(1:100,10,10)
10×10 reshape(::UnitRange{Int64}, 10, 10) with eltype Int64:
  1  11  21  31  41  51  61  71  81   91
  2  12  22  32  42  52  62  72  82   92
  3  13  23  33  43  53  63  73  83   93
  4  14  24  34  44  54  64  74  84   94
                     
  7  17  27  37  47  57  67  77  87   97
  8  18  28  38  48  58  68  78  88   98
  9  19  29  39  49  59  69  79  89   99
 10  20  30  40  50  60  70  80  90  100

julia> @view x[reshape(1:4,2,2)]
2×2 view(::UnitRange{Int64}, [1 3; 2 4]) with eltype Int64:
 1  3
 2  4

@maleadt
Copy link
Member

maleadt commented Dec 1, 2020

It may be in practice, but view with indices like that cannot be detected as such:

julia> W = @view x[reshape(1:4,2,2)]
2×2 view(::UnitRange{Int64}, [1 3; 2 4]) with eltype Int64:
 1  3
 2  4

julia> ans isa Base.FastContiguousSubArray
false

vs

julia> W = @view x[1:4]
4-element view(::UnitRange{Int64}, 1:4) with eltype Int64:
 1
 2
 3
 4

julia> ans isa Base.FastContiguousSubArray
true

@ChrisRackauckas
Copy link
Member Author

I think I can work around it: SciML/DiffEqFlux.jl#451

@ChrisRackauckas
Copy link
Member Author

Note that on CPUs, that other form is treated as a contiguous array by BLAS via JuliaLang/julia#34729

@maleadt
Copy link
Member

maleadt commented Dec 1, 2020

That's a good reference, thanks. I'm wary to copy it here though, because these unions have a tendency of wrecking package import latency...

@maleadt
Copy link
Member

maleadt commented Apr 27, 2024

The original MWE here seems to work now.

@maleadt maleadt closed this as completed Apr 27, 2024
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