search

FluxML / Functors.jl

Parameterise all the things
https://fluxml.ai/Functors.jl/stable/
MIT License
118 stars 15 forks source link

Fix recursion performance #61

Closed gaurav-arya closed 1 year ago

gaurav-arya commented 1 year ago

This PR simplifies the recursive structure of walking a functor. It uses a trick from https://github.com/JuliaLang/julia/issues/47760 to resolve https://github.com/FluxML/Functors.jl/issues/59.

The functionality of fmap after this PR is identical to its previous one. Thus, this PR lays bare the fact that the argument f is ultimately useless for the version of fmap that takes a user-specified walk. Note that this was public API, however, so I kept it in the signature and added an explanatory note.

Obviously the fmap API is highly imperfect, but I think this PR is a strict improvement, that simply lays bare some of the issues. (Ultimately, I think that the use of fmap with a user-specified arbitrary walk should probably be deprecated: walks are just way too flexible to allow arbitrary walks to be paired with fmap. The semantics of fmap seem to match best with the specific use case of ExcludeWalk: ExcludeWalk is what actually uses f, after all. The general function allowing for arbitrary walks should probably be called recursivewalk or something. But that would be very breaking.)

gaurav-arya commented 1 year ago

I don't believe this has got a release?

ToucheSir commented 1 year ago

I don't remember why. Might've held off because of the recent TagBot issues that prevented tags and release notes from being published. See https://github.com/JuliaRegistries/General/pull/77862