Interoperability with dual numbers

[SouthEndMusic]

I want to obtain the Jacobian sparsity of a function whose body contains a ForwardDiff.derivative call. This leads to this error:

ERROR: MethodError: *(::SparseConnectivityTracer.GradientTracer{...}, ::ForwardDiff.Dual{...}) is ambiguous.

Possible fix, define
  *(::SparseConnectivityTracer.GradientTracer, ::ForwardDiff.Dual{Ty}) where Ty

Does this have a quick fix?

Edit: I worked around it, so you can close the issue if you don't find this important.

[adrhill]

Maybe we're running into the issue described here: https://docs.julialang.org/en/v1/manual/methods/#Abstract-containers-and-element-types

[adrhill]

Replicated via

julia> using SparseConnectivityTracer, ForwardDiff

julia> using ForwardDiff: Dual, Partials, Tag

julia> d = Dual{Tag{*,Float64}}(1.2, 3.4)
Dual{Tag{*, Float64}}(1.2,3.4)

julia> jacobian_sparsity(x -> x*d, 1.0, TracerSparsityDetector())
ERROR: MethodError: *(::SparseConnectivityTracer.GradientTracer{…}, ::Dual{…}) is ambiguous.

Candidates:
  *(tx::SparseConnectivityTracer.GradientTracer, ::Real)
    @ SparseConnectivityTracer ~/Developer/SparseConnectivityTracer.jl/src/overloads/gradient_tracer.jl:125
  *(x::Real, y::Dual{Ty}) where Ty
    @ ForwardDiff ~/.julia/packages/ForwardDiff/PcZ48/src/dual.jl:145

Possible fix, define
  *(::SparseConnectivityTracer.GradientTracer, ::Dual{Ty}) where Ty
  
[...]

julia> jacobian_sparsity(x -> x*d, 1.0, TracerLocalSparsityDetector())
ERROR: MethodError: *(::SparseConnectivityTracer.Dual{Float64, SparseConnectivityTracer.GradientTracer{…}}, ::Dual{Tag{…}, Float64, 1}) is ambiguous.

Candidates:
  *(dx::D, y::Real) where {P, T<:SparseConnectivityTracer.GradientTracer, D<:SparseConnectivityTracer.Dual{P, T}}
    @ SparseConnectivityTracer ~/Developer/SparseConnectivityTracer.jl/src/overloads/gradient_tracer.jl:172
  *(x::Real, y::Dual{Ty}) where Ty
    @ ForwardDiff ~/.julia/packages/ForwardDiff/PcZ48/src/dual.jl:145

Possible fix, define
  *(::D, ::Dual{Ty}) where {Ty, P, T<:SparseConnectivityTracer.GradientTracer, D<:SparseConnectivityTracer.Dual{P, T}}
    
[...]

Looks like this is just a "regular" method ambiguity error.

[adrhill]

If this is a desired feature, we should rework the function overloading utilities for 2-to-1 functions to accept an optional type argument.

Then you could add a package extension for ForwardDiff and call something like

eval(SCT.generate_code_2_to_1(:Base, Dual, ops_2_to_1))

However, I'm not sure we can make this work in combination with other package extensions.

[gdalle]

Essentially the more general takeaway is that SCT doesn't play nice with other packages based on operator overloading

[gdalle]

Maybe for operations between a Tracer and a number we can handle things better through promotion? This is just a random thought, don't give it too much credit. I just know that I had encountered a similar issue with two operator overloading packages before (cjdoris/LogarithmicNumbers.jl#13), and the solution was promotion. But the error wasn't a method error so idk.

[adrhill]

Essentially the more general takeaway is that SCT doesn't play nice with other packages based on operator overloading

More generally packages implementing custom methods for 2-to-1 Base functions.

the solution was promotion. But the error wasn't a method error so idk.

I'm not sure how we would hit type promotion if the method call itself is ambiguous.
I'll open a draft PR and see how much complexity it adds.

[adrhill]

Basic operators from Julia Base should now work with ForwardDiff.Dual on main.
I'll tag a release tomorrow.

[adrhill]

After thinking about it for a day, I'm starting to think #200 shouldn't be released.
With #200, ForwardDiff.Dual are treated just like any other Real number, meaning that multiplication with a global tracer basically ignores the ForwardDiff.Dual. However, ForwardDiff.Dual could in theory contain SCT's tracers (either in the primal value, in the partials, or in both), thus returning wrong, non-conservative sparsity patterns.

While this is a general issue with Real number wrapper types, I feel like adding a ForwardDiff extension would signal full support for ForwardDiff.Dual. I therefore suggest reverting the changes in #200.

[gdalle]

I agree with that, good catch.

[adrhill]

#200 has been reverted. The newly tagged v0.6.6 release does not contain a ForwardDiff extension.