JuliaDiff / ForwardDiff.jl

Forward Mode Automatic Differentiation for Julia
Other
892 stars 148 forks source link

ChainRules pushforward on Partials sometimes returns Array #432

Open shashi opened 4 years ago

shashi commented 4 years ago
julia> x, pf = frule(sin, 0.0)
(0.0, ChainRules.var"#83#sin_pushforward#59"{Float64}(0.0))

julia> pf(Zero(), Partials((1.0,)))
1-element Partials{1,Float64}:
Error showing value of type Partials{1,Float64}:
ERROR: MethodError: no method matching getindex(::Tuple{Float64}, ::Int64, ::Int64)

yeah yeah show fails (trying to do 2d access??) but that's not what I want to debug.

using ModelingToolkit

ForwardDiff.can_dual(::Type{Expression}) = true
ForwardDiff.can_dual(::Type{<:Expression}) = true

@variables x y

julia> a, pf = frule(sin, x)
(sin(x), ChainRules.var"#83#sin_pushforward#59"{Operation}(x))

julia> pf(Zero(), Partials((ModelingToolkit.Constant(1),)))
1-element Array{Operation,1}:
 +(cos(x) * 1)

This is not a Partials array!! And a Dual(x, p) where p is a Partials array is not the same as when p is an Array, so this ruins everything.

I suspect this is a Partials promotion problem and less sure that it's a ChainRules problem, so I'm opening this here and pinging @oxinabox

shashi commented 4 years ago

Found an MWE

using ModelingToolkit
@variables x
julia> cos(x) * Partials((1,))
1-element Array{Operation,1}:
 cos(x) * 1
julia> 1.0 * Partials((1,))
1-element Partials{1,Float64}:
...
oxinabox commented 4 years ago

I think commutivity for scalar multiplication probably a requirement i am going to add to differentials, in ChainRules. Because noone wants to reason through this when writing rules. c.f. https://github.com/JuliaDiff/ChainRules.jl/pull/133