Return primal value in pushforward! and pullback!

[adrhill]

Adds two lower level functions:

value_and_pushforward!(dy, backend, f, x, dx) -> (y, dy)
value_and_pullback!(dx, backend, f, x, dx)    -> (y, dx)

that are called by pushforward! and pullback! and additionally return the primal value y = f(x).

[adrhill]

@gdalle any idea why BenchmarkCI fails to write a comment with the benchmark results?
tkf/BenchmarkCI.jl#97 looks related: https://github.com/gdalle/DifferentiationInterface.jl/actions/runs/7802170243/job/21278970728#step:7:14

[gdalle]

No the GitHub token has read and write permissions

[gdalle]

I think it might have the wrong permissions in your fork

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 84.00000% with 16 lines in your changes are missing coverage. Please review.

Project coverage is 82.90%. Comparing base (e37adca) to head (4f91f28).

Files	Patch %	Lines
ext/DifferentiationInterfaceChainRulesCoreExt.jl	50.00%	6 Missing ⚠️
src/backends.jl	60.00%	4 Missing ⚠️
src/forward.jl	0.00%	3 Missing ⚠️
src/reverse.jl	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #17      +/-   ##
==========================================
- Coverage   82.92%   82.90%   -0.03%     
==========================================
  Files           6        9       +3     
  Lines          82      117      +35     
==========================================
+ Hits           68       97      +29     
- Misses         14       20       +6     

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

[adrhill]

Running the benchmarks locally, I obtain:

  Results
  =======

  A ratio greater than 1.0 denotes a possible regression (marked with :x:), while a ratio less than 1.0 denotes a possible improvement (marked with :whitecheckmark:). Only significant results - results that
  indicate possible regressions or improvements - are shown below (thus, an empty table means that all benchmark results remained invariant between builds).

                                                             ID    time ratio  memory ratio
  ––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––– ––––––––––––– –––––––––––––
   ["forward", "scalar_to_vector", "10", "FiniteDiffBackend()"] 1.54 (5%) :x: 1.14 (1%) :x:
  ["forward", "scalar_to_vector", "10", "ForwardDiffBackend()"] 1.29 (5%) :x: 1.44 (1%) :x:
   ["forward", "vector_to_vector", "10", "FiniteDiffBackend()"] 1.20 (5%) :x: 1.30 (1%) :x:
  ["forward", "vector_to_vector", "10", "ForwardDiffBackend()"] 1.06 (5%) :x: 1.04 (1%) :x:
  ["reverse", "vector_to_scalar", "10", "ReverseDiffBackend()"] 1.22 (5%) :x: 1.34 (1%) :x:
  ["reverse", "vector_to_vector", "10", "ReverseDiffBackend()"] 1.14 (5%) :x: 1.09 (1%) :x:

[adrhill]

Looks like the use of DiffResults slowed down ForwardDiff and ReverseDiff due to some additional allocations.

All other backends return the primal result at no significant additional cost.

[gdalle]

I'll review next week sorry ☺️

[adrhill]

JET doesn't like the empty fallback functions that are only defined in the package extensions:

JET: JET-test failed at /home/runner/.julia/packages/JET/1ZQeA/src/JET.jl:1132
  Expression: (JET.report_package)(DifferentiationInterface; toplevel_logger = nothing, target_defined_modules = true)
  ═════ 2 possible errors found ═════
  ┌ pushforward!(dy::Any, backend::Any, f::Any, x::Any, dx::Any, stuff::Any) @ DifferentiationInterface /home/runner/work/DifferentiationInterface.jl/DifferentiationInterface.jl/src/forward.jl:33
  │ no matching method found `value_and_pushforward!(::Any, ::Any, ::Any, ::Any, ::Any, ::Any)`: value_and_pushforward!(dy, backend::Any, f::Any, x::Any, dx::Any, stuff::Any)
  └────────────────────
  ┌ pullback!(dx::Any, backend::Any, f::Any, x::Any, dy::Any, stuff::Any) @ DifferentiationInterface /home/runner/work/DifferentiationInterface.jl/DifferentiationInterface.jl/src/reverse.jl:33
  │ no matching method found `value_and_pullback!(::Any, ::Any, ::Any, ::Any, ::Any, ::Any)`: value_and_pullback!(dx, backend::Any, f::Any, x::Any, dy::Any, stuff::Any)
  └────────────────────

How do we best fix this?

[gdalle]

Just making a few tweaks and then I'll merge

[adrhill]

Both ForwardDiff and ReverseDiff reexport DiffResults, so I'm not sure the extra trigger is necessary.

• ForwardDiff: https://github.com/JuliaDiff/ForwardDiff.jl/blob/5e9240ae63e5d4217e1dc17cc14deca1d74c1ad7/src/ForwardDiff.jl#L29
• ReverseDiff: https://github.com/JuliaDiff/ReverseDiff.jl/blob/c982cde5494fc166965a9d04691f390d9e3073fd/src/ReverseDiff.jl#L61

[gdalle]

I know but it's another package and not a submodule, so it's clearer to me that way

Also they don't guarantee the re-export in the docs so theoretically it can disappear any moment

[adrhill]

The way I see things, it's a common submodule shared by Forward- and ReverseDiff. But I agree with the non-guaranteed re-export sentiment.
We should probably document that an explicit import of DiffResults is required for ForwardDiffBackend and ReverseDiffBackend.