search

JuliaDiff / AbstractDifferentiation.jl

An abstract interface for automatic differentiation.
https://juliadiff.org/AbstractDifferentiation.jl/
MIT License
137 stars 18 forks source link

Simpler defaults without FiniteDifferences special cases #96

Closed devmotion closed 1 year ago

devmotion commented 1 year ago

IMO the AbstractFiniteDifference special cases in the default definitions are confusing (see #94...) and lead to code that is less idiomatic and more difficult to optimize.

With increasing dimensionality, avoiding one additional computation of the primal matters less and less, and hence this PR proposes to disentangle the computation of the primal and the jacobian/Hessian/etc. Moreover, also with these changes one can improve performance for individual backends, if possible and desired, by defining a more optimized value_and_jacobian etc.

An additional advantage for higher-order calls value_and_hessian and value_gradient_and_hessian is that it is sufficient to call gradient instead of value_and_gradient.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: -1.30% :warning:

Comparison is base (3c18e86) 84.12% compared to head (4c55550) 82.82%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #96 +/- ## ========================================== - Coverage 84.12% 82.82% -1.30% ========================================== Files 8 8 Lines 466 425 -41 ========================================== - Hits 392 352 -40 + Misses 74 73 -1 ``` | [Files Changed](https://app.codecov.io/gh/JuliaDiff/AbstractDifferentiation.jl/pull/96?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaDiff) | Coverage Δ | | |---|---|---| | [src/AbstractDifferentiation.jl](https://app.codecov.io/gh/JuliaDiff/AbstractDifferentiation.jl/pull/96?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaDiff#diff-c3JjL0Fic3RyYWN0RGlmZmVyZW50aWF0aW9uLmps) | `78.18% <100.00%> (-0.28%)` | :arrow_down: | ... and [3 files with indirect coverage changes](https://app.codecov.io/gh/JuliaDiff/AbstractDifferentiation.jl/pull/96/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaDiff)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

gdalle commented 1 year ago

I'm strongly in favor of more simplicity at the expense of a little performance. I think there might be a conflict with #93 though, gonna try to do the merge

devmotion commented 1 year ago

Ah, missed that there's a merge conflict. I'll fix it.

devmotion commented 1 year ago

The merge conflicts are fixed, the PR is ready for review 🙂