ForwardDiff.jl icon indicating copy to clipboard operation
ForwardDiff.jl copied to clipboard

Add single-pass value_and_derivative(s) functions

Open gerlero opened this issue 1 year ago • 7 comments

Closes #610.

Proposal

I propose adding a value_and_derivative function for easy single-pass computation of value and derivative that has the same signature as derivative but returns a plain tuple. I also propose a value_and_derivatives function that also includes the second derivative. I'm not proposing methods for even higher order derivatives (honestly because I don't have a need for those), but if needed these could be added in the future via an optional argument (e.g. aVal{n}) in value_and_derivatives.

Rationale

The rationale is the one I presented in #610. To mention it here, using the DiffResults API (which is intended for these use cases) for very simple scalar derivatives is somewhat convoluted, crucially requiring knowledge of the return types of the differentiated function ahead of time instead of just being able to let the compiler deal with the types.

This has caused implementations of functions like value_and_derivative to appear in other packages (including mine); but these have to rely on internals of ForwardDiff (i.e., Dual and related functions), which is not ideal.

Naming

I considered the names value_and_derivative and value_derivative. I decided on the former as that's the name that was chosen in the AbstractDifferentiation package.

Background

Initially I wanted to register a wrapper package with this functionality, but Registry maintainers asked me to consider a PR here instead. I agree and I hope these changes can be merged here.

gerlero avatar Dec 19 '23 13:12 gerlero

CI failures seem to be unrelated to these changes

gerlero avatar Dec 19 '23 14:12 gerlero

I decided on the former as that's the name that was chosen in the AbstractDifferentiation package.

I wonder if AbstractDifferentation.value_and_derivative should be implemented in this way (which might make this PR obsolete?).

devmotion avatar Dec 19 '23 17:12 devmotion

I wonder if AbstractDifferentation.value_and_derivative should be implemented in this way (which might make this PR obsolete?).

If it's acceptable for that package to rely on these ForwardDiff internals, I don't mind making the change in there. Just, can we add a higher-order value_and_derivatives function to that package? AFAICT there's no way to get values and two derivatives in a single pass using only documented functions.

gerlero avatar Dec 19 '23 18:12 gerlero

Just, can we add a higher-order value_and_derivatives function to that package? AFAICT there's no way to get values and two derivatives in a single pass using only documented functions.

~~Sorry, I failed to notice that AsbtractDifferentiation already defines an interface for multiple derivatives (the signature is value_and_derivative(ab::AD.AbstractBackend, f, xs::Number...), BTW can this be type-stable?). So, this part could be ignored as ideally it would only require changes in the backend.~~

My bad. I got it wrong: that signature is for multiple function arguments; not for higher-order derivatives.

There's a value_gradient_and_hessian function though, so a value_and_derivatives function would fit right in there.

gerlero avatar Dec 19 '23 22:12 gerlero

I had a few minutes to play around with making these changes in AbstractDifferentiation instead (to complete the interface I also added second_derivative and value_and_second_derivative; which I guess I could add to this PR too).

While implementing the changes in AbstractDifferentiation is obviously viable, I have to say that that package's interface doesn't feel really comfortable when working with simple scalar functions (the nested tuple returns make debugging and getting it right in the first try more difficult). For this reason plus the already mentioned fact that the implementation relies on what are ForwardDiff internals, I'd really prefer that these functions be added here in ForwardDiff if at all possible.

gerlero avatar Dec 20 '23 12:12 gerlero

For this reason plus the already mentioned fact that the implementation relies on what are ForwardDiff internals, I'd really prefer that these functions be added here in ForwardDiff if at all possible.

Gentle bump @devmotion; are you willing to consider adding these functions to this package? (regardless of whether equivalent functions go into AbstractDifferentiation too)

gerlero avatar Jan 02 '24 12:01 gerlero

IMO there is no need for duplicate definitions, in particular given that the long-term goal of AbstractDifferentation is to move the extensions to the relevant packages (i.e., Zygote, Tracker, ForwardDiff etc.).

devmotion avatar Jan 03 '24 10:01 devmotion