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

ReverseDiff defines a huge number of methods. #226

Open
KristofferC opened this issue Apr 25, 2023 · 3 comments
Open

ReverseDiff defines a huge number of methods. #226

KristofferC opened this issue Apr 25, 2023 · 3 comments

Comments

@KristofferC
Copy link

KristofferC commented Apr 25, 2023

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
Copy link

@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
Copy link

prbzrg commented Oct 31, 2023

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
Copy link

prbzrg commented Oct 31, 2023

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.

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

3 participants