JuliaDiff / ReverseDiff.jl

Reverse Mode Automatic Differentiation for Julia
Other
348 stars 57 forks source link

ReverseDiff defines a huge number of methods. #226

Open KristofferC opened 1 year ago

KristofferC commented 1 year ago

Loading ReverseDiff and recording what methods are inserted we can see that this package defines a very large number of methods (number of definitions to the right inside parenthesis):

image

For example, there are almost a thousand definitions for materialize (which is not even something that should be extended according to the interface manual: https://docs.julialang.org/en/v1/manual/interfaces/#man-interfaces-broadcasting).

There are also many many for hcat, vcat.

Structuring the package like this heavily pessimize its load time and it would be good to be less "brute force" than this way where you just write a heavily nested loop and define methods in it.

Krastanov commented 1 year ago

@KristofferC For folks in the peanut gallery like me, could you let us know how you got this particular profile trace (list methods and the time it took to load them).

prbzrg commented 12 months ago

I think generalizing the methods definitions is the actual problem here.

julia> (xs::typeof(vcat))(x::String, s::String) = println("working")

julia> vcat("hi", "bye")
working

julia> (xs::Union{typeof(vcat), typeof(hcat)})(x::String, s::String) = println("working")
ERROR: Method dispatch is unimplemented currently for this method signature
Stacktrace:
 [1] top-level scope
   @ REPL[3]:1

If we could redirect a group of methods by arguments types, we wouldn't need to have a macro loop.

https://github.com/JuliaDiff/ReverseDiff.jl/blob/master/src/derivatives/arrays.jl#L51-L54

prbzrg commented 12 months ago

e.g. It would be:

(fun::AllArrayFunctionsType)(xs::Vararg{Union{Any, TrackedVecOrMat}}) = track(fun, xs...)

If any of the arguments are a TrackedVecOrMat it will be used but also if there isn't any suitable function for the arguments, it will be used too, so this isn't a good idea, but maybe there is a solution for it.