[AutoDiff] TF-{295,285}: Make '@differentiable' default parameters include 'self' on instance methods. #22877
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
@swift-ci please test tensorflow |
…clude 'self' on instance methods.
Since method differentiating w.r.t. `self` is more common than not w.r.t. `self` in an instance method, we make `@differentiable`'s implied parameter list include `self`.
```swift
protocol P : Differentiable {
@differentiable // equivalent to `@differentiable(wrt: (self, a, b))`
func foo(a: Float, b: Float) -> Float
}
```
No changes to static methods since metatypes are never differentiable.
* Imply `self` when type-checking a `@differentiable` that does not have a `wrt:` parameter list on an instance method.
* Fix the `@differentiable` protocol requirement matching logic to handle multiple attributes.
* Make `DifferentiableAttr::print` print parsed parameters instead of canonical parameter indices. This is important because protocol witness diagnostics are making use of this printout.
* Without a lot of special logic, canonical parameter indices would make things look like `@differentiable()`, `@differentiable(wrt: (x))`, etc, which are really not what we want in a diagnostic to encourage users to write. Parsed parameters are a lot simpler and intuitive: What you see in the protocol declaration is what you get in a diagnostic.
* Add more parameter-setting utilities to `AutoDiffParameterIndicesBuilder`.
Resolves [TF-295](https://bugs.swift.org/browse/TF-295) and [TF-285](https://bugs.swift.org/browse/TF-285).
Contributor
Author
|
@swift-ci please test tensorflow |
dan-zheng
approved these changes
Feb 25, 2019
Contributor
Author
|
@swift-ci please test tensorflow |
7 similar comments
Contributor
Author
|
@swift-ci please test tensorflow |
Contributor
Author
|
@swift-ci please test tensorflow |
Contributor
Author
|
@swift-ci please test tensorflow |
Contributor
Author
|
@swift-ci please test tensorflow |
Contributor
Author
|
@swift-ci please test tensorflow |
Contributor
Author
|
@swift-ci please test tensorflow |
Contributor
Author
|
@swift-ci please test tensorflow |
rxwei
added a commit
to tensorflow/swift-apis
that referenced
this pull request
Feb 26, 2019
…ed! (#33) We've recently changed the type checker to improve the ergonomics of the `@differentiable` attribute. * [swiftlang/swift#22915](swiftlang/swift#22915): Explicit differentiation parameters are no longer required in a `@differentiable` attribute when the function has some arguments that do not conform to `Differentiable`. Those non-differentiable parameters will be skipped and the rest will be differentiated with respect to. * [swiftlang/swift#22877](swiftlang/swift#22877): On an instance method, when a `wrt:` is not specified, `self` is being implicitly included as a differentiation parameter. * [swiftlang/swift#22877](swiftlang/swift#22877): When a `@differentiable` requirement is not met, the `@differentiable` attribute fix-it will appear exactly as written in the original declaration instead of the most complex, canonical form. For instance, `@differentiable` instead of `@differentiable(wrt: (x))`. This greatly simplifies libraries and applications that use automatic differentiation. The protocol requirement `Layer.applied(to:in:)` becomes as simple as this: ```swift @differentiable func applied(to input: Input, in context: Context) -> Output ``` This PR updates deep learning APIs to use the simplest form of `@differentiable` possible. Hooray!
rxwei
added a commit
to rxwei/swift
that referenced
this pull request
May 11, 2019
…clude 'self' on instance methods. (swiftlang#22877) * [AutoDiff] TF-{295,285}: Make '@differentiable' default parameters include 'self' on instance methods. Since method differentiating w.r.t. `self` is more common than not w.r.t. `self` in an instance method, we make `@differentiable`'s implied parameter list include `self`. ```swift protocol P : Differentiable { @differentiable // equivalent to `@differentiable(wrt: (self, a, b))` func foo(a: Float, b: Float) -> Float } ``` No changes to static methods since metatypes are never differentiable. * Imply `self` when type-checking a `@differentiable` that does not have a `wrt:` parameter list on an instance method. * Fix the `@differentiable` protocol requirement matching logic to handle multiple attributes. * Make `DifferentiableAttr::print` print parsed parameters instead of canonical parameter indices. This is important because protocol witness diagnostics are making use of this printout. * Without a lot of special logic, canonical parameter indices would make things look like `@differentiable()`, `@differentiable(wrt: (x))`, etc, which are really not what we want in a diagnostic to encourage users to write. Parsed parameters are a lot simpler and intuitive: What you see in the protocol declaration is what you get in a diagnostic. * Add more parameter-setting utilities to `AutoDiffParameterIndicesBuilder`. Resolves [TF-295](https://bugs.swift.org/browse/TF-295) and [TF-285](https://bugs.swift.org/browse/TF-285).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Semantic change
Since method differentiating w.r.t.
selfis more common than not w.r.t.selfin an instance method, we make@differentiable's implied parameter list includeself.No changes to static methods since metatypes are never differentiable.
Compiler changes
selfwhen type-checking a@differentiablethat does not have awrt:parameter list on an instance method.@differentiableprotocol requirement matching logic to handle multiple attributes.DifferentiableAttr::printprint parsed parameters instead of canonical parameter indices. This is important because protocol witness diagnostics are making use of this printout.@differentiable(),@differentiable(wrt: (x)), etc, which are really not what we want in a diagnostic to encourage users to write. Parsed parameters are a lot simpler and intuitive: What you see in the protocol declaration is what you get in a diagnostic.AutoDiffParameterIndicesBuilder.Resolves TF-295 and TF-285.