Skip to content

Tagging (fix #1296)#1297

Open
zjwegert wants to merge 4 commits intogridap:masterfrom
zjwegert:Tagging-fix
Open

Tagging (fix #1296)#1297
zjwegert wants to merge 4 commits intogridap:masterfrom
zjwegert:Tagging-fix

Conversation

@zjwegert
Copy link
Copy Markdown
Contributor

@zjwegert zjwegert commented May 4, 2026

Fixes and updates for #1296 as I don't have access to branch.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 4, 2026

Codecov Report

❌ Patch coverage is 80.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.84%. Comparing base (8e8a580) to head (05195b5).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
src/FESpaces/FEAutodiff.jl 66.66% 4 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1297   +/-   ##
=======================================
  Coverage   88.83%   88.84%           
=======================================
  Files         227      227           
  Lines       29780    29794   +14     
=======================================
+ Hits        26455    26469   +14     
  Misses       3325     3325           
Flag Coverage Δ
drivers 39.94% <30.00%> (-0.02%) ⬇️
extensions 5.12% <0.00%> (-0.01%) ⬇️
unit-adaptivity 40.08% <0.00%> (-0.02%) ⬇️
unit-basics 14.72% <40.00%> (+0.02%) ⬆️
unit-celldata 21.12% <0.00%> (-0.01%) ⬇️
unit-fespaces-1 32.35% <15.00%> (-0.02%) ⬇️
unit-fespaces-2 38.85% <60.00%> (+0.01%) ⬆️
unit-fields 17.61% <0.00%> (-0.01%) ⬇️
unit-geometry 28.76% <0.00%> (-0.02%) ⬇️
unit-multifield 30.88% <60.00%> (-0.01%) ⬇️
unit-odes 28.74% <15.00%> (-0.02%) ⬇️
unit-referencefes 34.24% <0.00%> (-0.02%) ⬇️
unit-visualization 11.91% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@zjwegert
Copy link
Copy Markdown
Contributor Author

zjwegert commented May 4, 2026

@JordiManyer This should be ready to go. Let me know if you want any changes

@JordiManyer
Copy link
Copy Markdown
Member

JordiManyer commented May 4, 2026

Has anyone though that compilation times for AD being so high is maybe because of these tags? Isn't there a better way of doing things?

Just to illustrate: By putting tags out of the inner functions and making them only once per call, we have reduced the multifield tests from 129 mins (master) down to 82 mins (in this PR). This is crazy, and we have basically only reduced the number of tags to one per AD call. Could we push this further?

For instance: I don't see why uh has to be embedded in the tag (maybe I am wrong, I have not thought this 100% through). Then we would specialize on AD function and weakform, which means consecutive calls to the same AD operation for different uh would not recompile (which they currently do). Again, maybe I am missing something, but it seems like it could work.

Going even further: Could we fully infer the "AD level" from the code, and then create a predictable, reusable tag? Something like

  struct FDTag{L} end 

FDTag(level::Integer) = FDTag{Val{level}}()

If we can reliably predict the level, this means we would only compile things once and thats it. It would also mean that we could precompile the code up to a certain level of AD and the precompiled code would be useful, not like now.

Does this make sense? @zjwegert @ConnorMallon

Comment thread src/Arrays/Autodiff.jl
autodiff_array_gradient(a,i_to_x,tag)
end

function autodiff_array_gradient(a,i_to_x,tag::Function)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In all functions: Could we unify into a single signature? Something like

function default_tag(f,a)
   tag = x -> f(a,x)
end

function autodiff_array_gradient(a,i_to_x,tag=default_tag(ForwardDiff.gradient,a))
  ...
end

Note it is still an argument, so the call to this function stays the same. I would also not restrict the tag to be a function, it could be anything.

Comment thread NEWS.md
Copy link
Copy Markdown
Collaborator

@Antoinemarteau Antoinemarteau May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"nested", not "tested"

@Antoinemarteau
Copy link
Copy Markdown
Collaborator

I was curious about these tags and investigated a bit. The "tag" is used as the first argument of ForwardDiff.GradientConfig, which wants the function you differentiate, its not just a matter of AD level ("perturbation confusion").

The docstring mentions the output of GradientConfig depends of the type of the function and not the function itself, so it might be possible to encapsulate a dictionary into this default_tag function to store only one anonymous function x-> gradient(f,x) per typeof(f).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants