Skip to content

Conversation

@pschuh
Copy link
Contributor

@pschuh pschuh commented Feb 26, 2019

This happens when inferring default 'wrt:' for @differentiable.

This should resolve: TF-296.

@pschuh pschuh requested a review from rxwei February 26, 2019 04:21
@pschuh
Copy link
Contributor Author

pschuh commented Feb 26, 2019

@swift-ci please test tensorflow

@rxwei rxwei requested a review from dan-zheng February 26, 2019 04:23
@rxwei
Copy link
Contributor

rxwei commented Feb 26, 2019

@dan-zheng is most familiar with this code path so I'll let him take a look first. My initial speculation is that the original attribute checking code was already checking Differentiable conformances, so the logic in this PR could be greatly simplified, but i could be wrong.

@rxwei
Copy link
Contributor

rxwei commented Feb 26, 2019

Update TypeCheckAttr to infer @nondiff for non-differentiable types.

By "infer @nondiff" do you just mean "infer what to not differentiate w.r.t."? The @nondiff parameter decl attribute (not the type attribute) is deprecated in favor of wrt:, and should be removed.

@pschuh pschuh changed the title Update TypeCheckAttr to infer @nondiff for non-differentiable types. Update TypeCheckAttr to infer what not to differentiate wrt for non-differentiable types. Feb 26, 2019
@pschuh
Copy link
Contributor Author

pschuh commented Feb 26, 2019

Perhaps? isDifferentiableParam is the only part that overlaps. I suppose I could make a helper lambda that does isDifferentiableParam but takes a bool whether or not to emit errors?

This happens when inferring default 'wrt:' for @differentiable.

This should resolve: TF-296.
@pschuh
Copy link
Contributor Author

pschuh commented Feb 26, 2019

@swift-ci please test tensorflow

2 similar comments
@pschuh
Copy link
Contributor Author

pschuh commented Feb 26, 2019

@swift-ci please test tensorflow

@pschuh
Copy link
Contributor Author

pschuh commented Feb 26, 2019

@swift-ci please test tensorflow

@rxwei rxwei merged commit 1db4e13 into swiftlang:tensorflow Feb 26, 2019
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 pushed a commit to rxwei/swift that referenced this pull request May 11, 2019
…wiftlang#22915)

This happens when inferring default 'wrt:' for @differentiable.

This should resolve: TF-296.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants