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

[AutoDiff] Rename @differentiating to @derivative(of:). #28481

Merged
merged 2 commits into from
Nov 26, 2019

Conversation

dan-zheng
Copy link
Contributor

@dan-zheng dan-zheng commented Nov 26, 2019

Rename @differentiating to @derivative(of:). @derivative(of:) more
clearly evokes derivative registration; the syntax is otherwise unchanged.

Deprecate @differentiating, to be removed in the next release.

Discussed here: #28321 (comment).

Partially resolves TF-999.
TF-1000 tracks updating all @differentiating usages across repositories.


Confirmed via grep -nr differentiating include lib stdlib test that all
remaining occurrences are intended.


Example:

extension Float {
    @derivative(of: +)
    static func vjpAdd(lhs: Self, rhs: Self) -> (value: Self, pullback: (Self) -> (Self, Self)) {
       (lhs + rhs, { v in (v, v) })
    }
}

Rename `@differentiating` to `@derivative(of:)`. `@derivative(of:)` more
clearly evokes derivative registration; the syntax is otherwise unchanged.

Deprecate `@differentiating`, to be removed in the next release.

Discussed here:
swiftlang#28321 (comment)

Partially resolves TF-999.
TF-1000 tracks updating all `@differentiating` usages across repositories.
@dan-zheng dan-zheng added the tensorflow This is for "tensorflow" branch PRs. label Nov 26, 2019

/// SWIFT_ENABLE_TENSORFLOW
ParserResult<DerivativeAttr>
Parser::parseDifferentiatingAttribute(SourceLoc atLoc, SourceLoc loc) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I duplicated parseDifferentiatingAttribute rather than adding a flag to parseDerivativeAttribute to be future-facing. It's easier to remove duplicated code.

I avoided unnecessary/duplicate code in other places (e.g. not defining a new DifferentiatingAttr class).

"'@differentiating' attribute requires function to return a two-element tuple of type "
// @derivative
ERROR(derivative_attr_expected_result_tuple,none,
"'@derivative(of:)' attribute requires function to return a two-element tuple of type "
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One topic to discuss is whether to show @derivative(of:) or @derivative in user-facing messages.
I chose @derivative(of:), following the precedent of dynamicReplacement(for:).

Code comments mostly use @derivative without the label.

Copy link
Contributor

@rxwei rxwei Nov 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dynamicReplacement(for:) doesn't take additional arguments like we do, but @derivative(of:wrt:) is not good either. This works for me.

@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow

@rxwei
Copy link
Contributor

rxwei commented Nov 26, 2019

I'll merge this in order to get SR-11849 rolling.

@rxwei rxwei merged commit b935486 into swiftlang:tensorflow Nov 26, 2019
@dan-zheng dan-zheng deleted the rename-derivative-attr branch November 26, 2019 13:54
dan-zheng added a commit to dan-zheng/swift-apis that referenced this pull request Nov 27, 2019
dan-zheng added a commit to tensorflow/swift-apis that referenced this pull request Nov 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tensorflow This is for "tensorflow" branch PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants