-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[AutoDiff] switch array and floating point derivatives to @derivative(of:) #29030
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Minor comment: The title initially came across as "removing VJPs from stdlib", and I only realized it's removing @differentiable(vjp: ...)
s when I looked at the diff. How about rewording the PR title to something like "[AutoDiff] Switch to '@Derivative(of:)' for array and floating point derivative registrations"?
@swift-ci please test tensorflow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it (or will it eventually be) necessary to keep @differentiable
attributes on the public
original functions so that the derivatives are registered with public
access?
We discussed this as part of derivative visibility. Currently, derivative visibility is equal to the access level of the @derivative
declaration, but we could potentially decouple it (e.g. by adding syntax like @derivative(of: foo, access: public)
).
We discussed this and preferred making the derivative registration have the same visibility as the derivative function. The only thing that's preventing this is the fact that we do not yet support anonymous function declarations (with The current code pattern (as it is in the library) will likely not compile in the final design, because when |
76bdf43
to
0cd6e2a
Compare
@swift-ci please test tensorflow |
4 similar comments
@swift-ci please test tensorflow |
@swift-ci please test tensorflow |
@swift-ci please test tensorflow |
@swift-ci please test tensorflow |
Also I added a test for the
FloatingPoint.swift
derivatives because there isn't an existing test for them.Once this and #28933 are merged, there will be no more vjp:s in the stdlib.