-
Notifications
You must be signed in to change notification settings - Fork 18
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
Get rid of ugly macro? #123
Comments
To elaborate on this: What the macro doesAs far as I understand, the Forward-mode AD: AbstractDifferentiation.jl/src/AbstractDifferentiation.jl Lines 600 to 634 in 211b675
Reverse-mode AD: AbstractDifferentiation.jl/src/AbstractDifferentiation.jl Lines 636 to 663 in 211b675
These functions compute full Jacobians by evaluating the pullbacks/pushforwards on the standard basis ( Fallback behaviorBy default, the fallback
As shown in the implementer guide, this Taking reverse-mode AD as an example, the function dependency graph of
Now, when a reverse-mode AD backend is loaded,
The second behaviour is desired, as we wouldn't want to compute a full Jacobian just to compute a VJP when we can instead evaluate the pullback directly. The fact that the function dependency graph is flipped was very confusing to me at first. A lot of hidden control flow is added via package extensions and the Back to the questionWhy is The only advantage I currently see is to allow users to
but those sound like things that should usually be avoided. Why isn't AbstractDifferentiation.jl built around two primitives Footnotes
|
Duplicate of #13, or at least #13 (comment) and the following discussion? |
Historical reasons based mainly on the original author have a strong enough understanding of the calculus involved, but not such a strong understanding of autodiff or julia abstractions, IIRC. And the priority being on getting something out that worked and was usable. It should be. |
This issue is my fault. Feel free to remove the macro if it makes things simpler. |
BTW, regarding
#95 trimmed down the macro, it can only be used anymore to implement the jacobian based on a pushforward_function or a value_and_pullback_function. Support for ReverseDiff and FiniteDifferences is implemented without the macro already, and e.g. ForwardDiff uses the automatically constructed jacobian function only for functions with multiple arguments (the single-argument version just calls |
As I mentioned in #13 (comment) and #123 (comment), I am ok with removing the macro. It is currently a thin wrapper over a pushforward or pullback definition. Feel free to open a PR. |
I was chatting with @adrhill and he suggested that the macro
@primitive
could be discarded if each backend simply implemented some methods from AbstractDifferentiation, mostlyjacobian
and apushforward
orpullback
. Thoughts?The text was updated successfully, but these errors were encountered: