Simpler defaults without FiniteDifferences special cases#96
Simpler defaults without FiniteDifferences special cases#96
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ 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
☔ View full report in Codecov by Sentry. |
|
I'm strongly in favor of more simplicity at the expense of a little performance. |
|
Ah, missed that there's a merge conflict. I'll fix it. |
|
The merge conflicts are fixed, the PR is ready for review 🙂 |
gdalle
left a comment
There was a problem hiding this comment.
Looks good to me, apart from the deletion in the @primitive macro, which I don't understand.
Although to be fair I don't understand the macro as a whole. Why do we only have 2 out of 5 (now 4) ..._and_friends functions defined in the package? Where are the rest?
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_jacobianetc.An additional advantage for higher-order calls
value_and_hessianandvalue_gradient_and_hessianis that it is sufficient to callgradientinstead ofvalue_and_gradient.