Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bad interaction with StaticArrays broadcast #64

Closed
tkoolen opened this issue Mar 29, 2017 · 3 comments
Closed

Bad interaction with StaticArrays broadcast #64

tkoolen opened this issue Mar 29, 2017 · 3 comments

Comments

@tkoolen
Copy link

tkoolen commented Mar 29, 2017

I was trying out using ReverseDiff with my package, which relies heavily on StaticArrays. Unfortunately, I ran into an issue with broadcast:

using ReverseDiff: GradientTape
using StaticArrays

function f(x)
    T = eltype(x)
    v = SVector(one(T), zero(T))
    sum(sum(x) * v)
end
f_tape = GradientTape(f, rand(1))

results in the following ambiguity error on 0.6 with StaticArrays 0.0.4 and ReverseDiff 0.1.2:

ERROR: MethodError: broadcast(::Base.#*, ::ReverseDiff.TrackedReal{Float64,Float64,Void}, ::StaticArrays.SVector{2,ReverseDiff.TrackedReal{Float64,Float64,ReverseDiff.TrackedArray{Float64,Float64,1,Array{Float64,1},Array{Float64,1}}}}) is ambiguous. Candidates:
  broadcast(f, a::Union{Number, StaticArrays.StaticArray}...) in StaticArrays at /home/twan/code/RigidBodyDynamics/v0.6/StaticArrays/src/broadcast.jl:8
  broadcast(::Base.#*, x::ReverseDiff.TrackedReal{X,D,O} where O, y::AbstractArray{T,1} where T) where {X, D} in ReverseDiff at /home/twan/code/RigidBodyDynamics/v0.6/ReverseDiff/src/derivatives/elementwise.jl:344
Stacktrace:
 [1] f(::ReverseDiff.TrackedArray{Float64,Float64,1,Array{Float64,1},Array{Float64,1}}) at ./REPL[7]:4
 [2] Type at /home/twan/code/RigidBodyDynamics/v0.6/ReverseDiff/src/api/tape.jl:176 [inlined]
 [3] ReverseDiff.GradientTape(::Function, ::Array{Float64,1}) at /home/twan/code/RigidBodyDynamics/v0.6/ReverseDiff/src/api/tape.jl:175

The relevant methods are:

How do you think this should be resolved?

@jrevels
Copy link
Member

jrevels commented Mar 29, 2017

Ah, that's pretty painful. Naively, I guess the easiest solution is to add StaticArrays types here.

I think StaticArrays is probably popular enough at this point that JuliaDiff should have a "StaticArraysDiff" package (or whatever) that provides a bunch of StaticArrays-specialized ForwardDiff/ReverseDiff methods. See also JuliaDiff/ForwardDiff.jl#208 (comment).

Even beyond StaticArrays, though, I imagine it would be pretty common for external array types to introduce ambiguities w.r.t. ReverseDiff methods. We should come up with a convenient process by which users can dynamically perform the equivalent of adding such array types to the aforementioned list and generating new methods.

It seems to me like this kind of compatibility problem would arise any time somebody tries to compose two sufficiently complex/specialized non-Base array types. I wonder how (or if) other folks have resolved it...might make a good post for Discourse.

@tkoolen
Copy link
Author

tkoolen commented Mar 29, 2017

Thanks for the quick response and workaround. I'll post on Discourse, but first I'll play around with ReverseDiff a little more.

@tkoolen
Copy link
Author

tkoolen commented Apr 10, 2017

Just got back to this after working on some other stuff. I realize now that the bigger problem with StaticArrays and ReverseDiff is that StaticArrays are immutable, while ReverseDiff I guess expects non-input arrays used in the function to be mutable, correct? The ambiguity issues are still valid, but since you can't actually use StaticArrays with ReverseDiff anyway, I'll close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants