search

jonniedie / ComponentArrays.jl

Arrays with arbitrarily nested named components.
MIT License
286 stars 34 forks source link

Fix Zygote accum for CA in GPU broadcasting #221

Closed avik-pal closed 10 months ago

avik-pal commented 10 months ago

Continuing my saga of upstreaming pirated patches from Lux. Zygote + AMDGPU + CAs.jl don't play too well. See https://buildkite.com/julialang/lux-dot-jl/builds/634#018ab9ff-bbcb-47bf-aa05-079929053253/6-4602.

This is mostly a hacky patch, but it fixes it.

codecov-commenter commented 10 months ago

Codecov Report

Merging #221 (b269890) into main (140b899) will increase coverage by 0.07%. The diff coverage is 100.00%.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##             main     #221      +/-   ##
==========================================
+ Coverage   73.16%   73.24%   +0.07%     
==========================================
  Files          22       23       +1     
  Lines         723      725       +2     
==========================================
+ Hits          529      531       +2     
  Misses        194      194
Files Changed Coverage Δ
ext/ComponentArraysZygoteExt.jl 100.00% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

jonniedie commented 10 months ago

Related to #186, I think

avik-pal commented 10 months ago

Related to https://github.com/jonniedie/ComponentArrays.jl/issues/186, I think

Not really. I think it is mostly on AMDGPU side not being very robust to broadcasting. I have faced it in places completely detached from Zygote and CAs