ForwardDiff.jl icon indicating copy to clipboard operation
ForwardDiff.jl copied to clipboard

add back the iteration based seeding for `Array`

Open KristofferC opened this issue 2 years ago • 2 comments

These were removed in https://github.com/JuliaDiff/ForwardDiff.jl/pull/472 and while I was a bit uncomfortable with it I did not protest. However, now after doing some benchmarks I see that this has a runtime cost that is very much nonsignificant.

Below is a profile and the big peaks are from seed!.

gnome-shell-screenshot-57lfk9

If I use the iteration-based seed function they completely disappear. In order to not break anything, I added these back but with a restriction to Array so that e.g. GPU arrays will keep using the broadcast based seeding.

KristofferC avatar Mar 09 '23 13:03 KristofferC

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.32 :tada:

Comparison is base (1592fe9) 87.28% compared to head (bafdb3b) 87.60%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #634      +/-   ##
==========================================
+ Coverage   87.28%   87.60%   +0.32%     
==========================================
  Files          10       10              
  Lines         912      936      +24     
==========================================
+ Hits          796      820      +24     
  Misses        116      116              
Impacted Files Coverage Δ
src/apiutils.jl 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov[bot] avatar Mar 09 '23 14:03 codecov[bot]

I wonder if one could use map! instead of broadcasting? I would assume that this would still be GPU-compatible but possibly there is (almost) no runtime cost for Array?

devmotion avatar Mar 09 '23 14:03 devmotion