Ambiguity error for `rev` with latest version of Symbolics

[ChrisRackauckas]

What's the error?

[dpsanders]

@ChrisRackauckas:

Running the second example in the README gives the following:

julia> vars = @variables x, y 

julia> ex = x^2 + y^2

julia> C = forward_backward_contractor(ex, vars)  # construct the contractor
ERROR: MethodError: rev(::typeof(+), ::Num, ::Num, ::Num) is ambiguous. Candidates:
  rev(arg1, arg2::Num, arg3::Num, arg4::Num) in ReversePropagation at /Users/dsanders/.julia/packages/Symbolics/LfLYY/src/register.jl:51
  rev(arg1, arg2::Num, arg3::Num, arg4::Real) in ReversePropagation at /Users/dsanders/.julia/packages/Symbolics/LfLYY/src/register.jl:51
  rev(::typeof(+), z::Num, x::Real, y::Num) in ReversePropagation at /Users/dsanders/Dropbox/packages/ReversePropagation/src/reverse_icp.jl:63
  rev(arg1, arg2::Num, arg3::Real, arg4::Num) in ReversePropagation at /Users/dsanders/.julia/packages/Symbolics/LfLYY/src/register.jl:51
  rev(arg1, arg2::Num, arg3::Real, arg4::Real) in ReversePropagation at /Users/dsanders/.julia/packages/Symbolics/LfLYY/src/register.jl:51
  rev(::typeof(+), z::Real, x::Num, y::Num) in ReversePropagation at /Users/dsanders/Dropbox/packages/ReversePropagation/src/reverse_icp.jl:64
  rev(arg1, arg2::Real, arg3::Num, arg4::Num) in ReversePropagation at /Users/dsanders/.julia/packages/Symbolics/LfLYY/src/register.jl:51
  rev(arg1, arg2::Real, arg3::Real, arg4::Num) in ReversePropagation at /Users/dsanders/.julia/packages/Symbolics/LfLYY/src/register.jl:51
  rev(arg1, arg2::Real, arg3::Num, arg4::Real) in ReversePropagation at /Users/dsanders/.julia/packages/Symbolics/LfLYY/src/register.jl:51
  rev(::typeof(+), z::Real, x::Real, y::Real) in ReversePropagation at /Users/dsanders/Dropbox/packages/ReversePropagation/src/reverse_icp.jl:62
Possible fix, define
  rev(::typeof(+), ::Num, ::Num, ::Num)
Stacktrace:
  [1] rev(eq::SymbolicUtils.Code.Assignment, params::Vector{Any})
    @ ReversePropagation ~/Dropbox/packages/ReversePropagation/src/reverse_icp.jl:33
  [2] _broadcast_getindex_evalf
    @ ./broadcast.jl:670 [inlined]
  [3] _broadcast_getindex
    @ ./broadcast.jl:643 [inlined]
  [4] getindex
    @ ./broadcast.jl:597 [inlined]
  [5] macro expansion
    @ ./broadcast.jl:961 [inlined]
  [6] macro expansion
    @ ./simdloop.jl:77 [inlined]
  [7] copyto!
    @ ./broadcast.jl:960 [inlined]
  [8] copyto!
    @ ./broadcast.jl:913 [inlined]
  [9] copy
    @ ./broadcast.jl:885 [inlined]
 [10] materialize
    @ ./broadcast.jl:860 [inlined]
 [11] forward_backward_code(ex::Num, vars::Vector{Num}, params::Vector{Any})
    @ ReversePropagation ~/Dropbox/packages/ReversePropagation/src/reverse_icp.jl:100
 [12] forward_backward_expr(ex::Num, vars::Vector{Num}, params::Vector{Any})
    @ ReversePropagation ~/Dropbox/packages/ReversePropagation/src/reverse_icp.jl:139
 [13] forward_backward_contractor_code(ex::Num, vars::Vector{Num}, params::Vector{Any})
    @ ReversePropagation ~/Dropbox/packages/ReversePropagation/src/reverse_icp.jl:155
 [14] forward_backward_contractor (repeats 2 times)
    @ ~/Dropbox/packages/ReversePropagation/src/reverse_icp.jl:190 [inlined]
 [15] top-level scope
    @ REPL[37]:1

[dpsanders]

Doing

@register rev(z::typeof(+), a, b, c)

seems to fix the problem.

[ChrisRackauckas]

@shashi is this fixed by the register patch?

[shashi]

Doing

@register rev(z::typeof(+), a, b, c)

seems to fix the problem.

That is indeed the correct way to use it? Also, it's renamed @register_symbolic to indicate that it does not override existing methods, only works on symbolic input. What were you doing before?

[shashi]

I see you want to capture any function in place of z and rev has other methods specializing the first argument...

These methods can indeed be ambiguous, so every place you define a specialization for rev for a specific function, you may also want to register the same. This will make sure there are no ambiguities.

[dpsanders]

Thanks @shashi; sorry, I only just saw your messages for some reason. This used to work but I see your point.

[lucaferranti]

with #66 it passes the tests, but when using ReversePropagation there are a bunch of warning that incremental compilation is broken due to method redefinition, not sure what is causing it