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

Implement partials/value for complex duals

Open dlfivefifty opened this issue 1 year ago • 5 comments

At the moment values/partials give useless (and probably "wrong") answers for complex-duals:

julia> ForwardDiff.value(ForwardDiff.Dual(1,2) + im*ForwardDiff.Dual(3,4))
Dual{Nothing}(1,2) + Dual{Nothing}(3,4)*im


julia> ForwardDiff.value(ForwardDiff.Dual(1,2) + im*ForwardDiff.Dual(3,4))
Dual{Nothing}(1,2) + Dual{Nothing}(3,4)*im

julia>  ForwardDiff.partials(ForwardDiff.Dual(1,2) + im*ForwardDiff.Dual(3,4))
0-element ForwardDiff.Partials{0, Complex{ForwardDiff.Dual{Nothing, Int64, 1}}}

julia>  ForwardDiff.partials(ForwardDiff.Dual(1,2) + im*ForwardDiff.Dual(3,4),1)
Dual{Nothing}(0,0) + Dual{Nothing}(0,0)*im

This PR implements correct versions of these functions.

dlfivefifty avatar Dec 12 '24 21:12 dlfivefifty

Are these the desired return types?

julia> ForwardDiff.partials(ForwardDiff.Dual(1,2) + im*ForwardDiff.Dual(3,4))
1-element Vector{Complex{Int64}}:
 2 + 4im

julia> ForwardDiff.partials(1 + im)
0-element ForwardDiff.Partials{0, Complex{Int64}}

One alternative is

julia> @eval ForwardDiff @inline partials(d::Complex{<:Dual}) = Partials(map(complex, d.re.partials.values, d.im.partials.values))
partials (generic function with 13 methods)

julia> ForwardDiff.partials(ForwardDiff.Dual(1,2) + im*ForwardDiff.Dual(3,4))
1-element Partials{1, Complex{Int64}}:
 2 + 4im

mcabbott avatar Dec 12 '24 21:12 mcabbott

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 85.90%. Comparing base (c310fb5) to head (d05bb57). Report is 13 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #732      +/-   ##
==========================================
- Coverage   89.57%   85.90%   -3.68%     
==========================================
  Files          11       10       -1     
  Lines         969      922      -47     
==========================================
- Hits          868      792      -76     
- Misses        101      130      +29     

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

codecov[bot] avatar Dec 12 '24 21:12 codecov[bot]

Ah yes you are right, it is better to return a Partials. It's fixed now

dlfivefifty avatar Dec 12 '24 21:12 dlfivefifty

I don't swear this is better, it just needs some thought. Are there any paths by which Partials{3, ComplexF64} can result in a Dual{Complex} instead of Complex{Dual}? That would be bad.

mcabbott avatar Dec 12 '24 22:12 mcabbott

Given that nothing is currently calling partials(::Complex) I don't see how this change can result in that happening, but yes there is a question about what Partials can represent.

dlfivefifty avatar Dec 12 '24 22:12 dlfivefifty