-
Notifications
You must be signed in to change notification settings - Fork 80
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
use style dispatch in materialize #295
Conversation
What's the advantage of this? Tests? |
|
It seems like this is actually needed to get nice interoperability between StructArrays broadcast and GPU arrays broadcast, see JuliaArrays/StructArrays.jl#211 (comment). A This change (plus potentially moving the |
@piever want to take a stab at this? As you can tell I run out if time to work on it shortly after opening the PR |
@vchuravy I guess the following is enough? @inline function materialize!(::Style, dest, bc::Broadcasted) where {Style<:AbstractGPUArrayStyle}
return _copyto!(dest, instantiate(Broadcasted{Style}(bc.f, bc.args, axes(dest))))
end
@inline Base.copyto!(dest::BroadcastGPUArray, bc::Broadcasted{Nothing}) = _copyto!(dest, bc) # Keep it for ArrayConflict
@inline Base.copyto!(dest::AbstractArray, bc::Broadcasted{<:AbstractGPUArrayStyle}) = _copyto!(dest, bc)
@inline function _copyto!(dest::AbstractArray, bc::Broadcasted)
axes(dest) == axes(bc) || Broadcast.throwdm(axes(dest), axes(bc))
isempty(dest) && return dest
bc′ = Broadcast.preprocess(dest, bc)
... Local test shows no problem. Is it OK to open a new PR? |
@N5N3, I think it would be great if you could open PRs here and on StructArrays to check that everything works. I confess I am a bit out of my depth on the technicalities of the |
Superseded by #393 IIUC |
Supersedes #295, relies on JuliaLang/julia#35620.
x-ref: JuliaLang/julia#35620