rewrite make_zero! and remake_zero! with ntuple over fields#2607
rewrite make_zero! and remake_zero! with ntuple over fields#2607
Conversation
Benchmark Results
Benchmark PlotsA plot of the benchmark results has been uploaded as an artifact at https://github.com/EnzymeAD/Enzyme.jl/actions/runs/18039430455/artifacts/4115489625. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2607 +/- ##
=======================================
Coverage 75.30% 75.30%
=======================================
Files 57 57
Lines 17751 17751
=======================================
Hits 13368 13368
Misses 4383 4383 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
af38ba4 to
bcf210e
Compare
| end | ||
| push!(seen, prev) | ||
| for i in 1:nf | ||
| ntuple(Val(nf)) do i |
There was a problem hiding this comment.
honestly this feels like it should become a generated function (and we can also therefore conditionally create the seen dict if there is > 1 potentially aliasing piece of data)
There was a problem hiding this comment.
ntuple is a generated function, just one that is limited in scope. I still prefer this over the headache that generated function bring with them otherwise.
I also don't think it would be unreasonable for a user to provide their own make_zero function / remake_zero! function.
There was a problem hiding this comment.
yeah we definitely want user definable overrides. I'm just dreaming of a world where we don't need the explicit specialization for make_zero(Array) in addition to make_zero(Array, seen).
in any case we can figure this out later, this is already clearly a good change
There was a problem hiding this comment.
Also sadly while we might be able to use a generated function, that would only allow us to guard against recursion in the type-domain and not recursion in the value domain :/
There was a problem hiding this comment.
Perhaps but things like make_zero of a tuple of floats is sufficiently common that we really want it to use the right thing without the is check
3e17451 to
8f268dc
Compare
8f268dc to
d4f6bf6
Compare
No description provided.