ReverseDiff.jl icon indicating copy to clipboard operation
ReverseDiff.jl copied to clipboard

ReverseDiff defines a huge number of methods.

Open KristofferC opened this issue 2 years ago • 3 comments

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.

KristofferC avatar Apr 25 '23 16:04 KristofferC

@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).

Krastanov avatar Jul 17 '23 16:07 Krastanov

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 avatar Oct 31 '23 07:10 prbzrg

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.

prbzrg avatar Oct 31 '23 07:10 prbzrg