Broadcasted setindex! triggers scalar setindex! #101
Comments
|
Looks like it can be even simpler: using CuArrays
CuArrays.allowscalar(false)
J = cu(zeros(30,30))
setindex!.((J,),1.0,1:30,1:30) |
ChrisRackauckas
changed the title
|
Yes, this is expected. Broadcast descends into all arguments, including the If you prevent broadcast from expanding the ranges, everything works: The outer broadcast itself can't be executed on the GPU because you're essentially doing a host-side broadcast on a tuple of arrays. |
But it's a broadcast expression, so theoretically this could've built a kernel instead of 1 by 1?
|
But it doesn't on the CPU either? |
|
I guess I simplified https://github.com/JuliaDiff/SparseDiffTools.jl/blob/master/src/differentiation/compute_jacobian_ad.jl#L192 a little bit too much. But that will still scalar index because it's broadcasting get/set index? |
|
Ah, looking at that code I understand what you're getting at. That tuple threw me off, why don't you just |
|
I think this is fixable. Will have a look tomorrow. |
|
I'm not going to be able to fix this completely, because Causing the broadcast to return an array of arrays: (even if the 0D broadcast were to unwrap, which I thought it should, it would need to save the output of that one setindex! somewhere leading to the very same problem) Can make this work by locally defining a |
That works for me. Would we have to locally use |
Neither did I, especially since assignment has different semantics: julia> a = [1]
1-element Array{Int64,1}:
1
julia> typeof(begin a[1] = 2 end)
Int64
julia> typeof(setindex!(a, 2, 1))
Array{Int64,1}
julia> Meta.@lower a[1] = 2
:($(Expr(:thunk, CodeInfo(
@ none within `top-level scope'
1 ─ Base.setindex!(a, 2, 1)
└── return 2
))))You'd have to define a |
|
I'll just add void_setindex!(args...) = (setindex!(args...); nothing)? |
|
Yes, but wait a while before finalizing on that, I'm still in the process of fixing the GPUArrays broadcast machinery to make this possible in the first place. |
|
The following works now: But you need GPUArrays/CuArrays/CUDAnative from master. |
With https://github.com/JuliaGPU/CuArrays.jl/issues/571 fixed it should allow the GPU sparse tests to pass
|
Might still be worth some feedback from broadcast experts though, maybe cc @mbauman? Specifically, https://github.com/JuliaGPU/CuArrays.jl/issues/571#issuecomment-580776718, where returning an array of arrays breaks GPU support. Any way to avoid that with the current design, or suggestions on how to make that possible? |
|
Looks like this is fine on Ref, but breaks when using tuples to stop the broadcast: using CuArrays, GPUArrays
a = [1]
A = CuArray(a)
void(f) = (args...)->(f(args...); return)
@. void(setindex!).(Ref(a),1)
@. void(setindex!).(Ref(A),1)
@. void(setindex!).((a,),1)
@. void(setindex!).((A,),1)Note that tuples are recommended: JuliaLang/julia#35591 And the actual application seems to not work: JuliaDiff/SparseDiffTools.jl#104 |
maleadt
added
cuda array
|
So the reason that tuples aren't working is that CUDA isn't grabbing tuples of CuArrays as being a julia> typeof(Broadcast.broadcasted(void(setindex!), Ref(A), 1))
Base.Broadcast.Broadcasted{CUDA.CuArrayStyle{0}, Nothing, var"#1#2"{typeof(setindex!)}, Tuple{Base.RefValue{CuArray{Int64, 1, CUDA.Mem.DeviceBuffer}}, Int64}}
julia> typeof(Broadcast.broadcasted(void(setindex!), (A,), 1))
Base.Broadcast.Broadcasted{Base.Broadcast.Style{Tuple}, Nothing, var"#1#2"{typeof(setindex!)}, Tuple{Tuple{CuArray{Int64, 1, CUDA.Mem.DeviceBuffer}}, Int64}}I'm not sure off-hand what the magic would be to ensure that |
|
Why does |
Because we special-case it here:
Right, but we don't want that or the following starts to return a device array: julia> BroadcastStyle(::Type{Tuple{AT}}) where {AT<:AbstractGPUArray} = typeof(BroadcastStyle(AT))(Val(1))
julia> julia> size.((CUDA.rand(1),))
1-element CuArray{Tuple{Int64}, 1, CUDA.Mem.DeviceBuffer}:
(1,) |
|
Looks like this has adequate solutions now. |
The text was updated successfully, but these errors were encountered: