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] [stdlib] Deprecate 'CotangentVector' in favor of 'TangentVector'. #24825

Merged
merged 3 commits into from
May 16, 2019

Conversation

rxwei
Copy link
Contributor

@rxwei rxwei commented May 16, 2019

This PR removes the CotangentVector associated type and make it equal to TangentVector.

Mathematically, types conforming to the Differentiable protocol represent a Riemannian manifold, whose metric is inner products, which provide an isomorphism between a tangent space and a cotangent space at a point.

Some theoretical and practical reasons:

  • It is not possible to correctly represent dual vectors in Swift today because functions cannot conform to protocols ((TangentVector) -> Scalar cannot conform to AdditiveArithmetic).
  • In computation, it is always more practical to compute numerical tangent vectors instead of true cotangent vectors (partially applied dot product functions).
  • The mutually recursive generic constraints triggered a significant bug in the type checker (SR-9595) that required a lot of workarounds (TF-213), in particular, an ugly split of 3 protocols: __Differentiable, _Differentiable, and Differentiable.
  • The split between TangentVector and CotangentVector makes user-defined conformances complicated.

Changes include:

  • Remove associatedtype CotangentVector from Differentiable.
  • Define typealias CotangentVector = TangentVector in Differentiable with a deprecation message.
  • Merge __Differentiable and _Differentiable back into Differentiable.
  • Replace all mentions of "cotangent" with "tangent" in the code base.
  • Adapt derived conformances logic.
  • Adapt tests.

Defines away TF-213.

@rxwei rxwei added the tensorflow This is for "tensorflow" branch PRs. label May 16, 2019
@rxwei rxwei requested review from dan-zheng and marcrasi May 16, 2019 10:07
@rxwei
Copy link
Contributor Author

rxwei commented May 16, 2019

@swift-ci please test tensorflow

lib/Sema/TypeCheckProtocol.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@dan-zheng dan-zheng left a comment

Choose a reason for hiding this comment

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

Exciting simplification!
I skimmed the files, LGTM if tests pass.

@rxwei
Copy link
Contributor Author

rxwei commented May 16, 2019

@swift-ci please clean test tensorflow Linux

@rxwei
Copy link
Contributor Author

rxwei commented May 16, 2019

All tests pass locally, but CI seems dead.

@rxwei rxwei merged commit e9aa913 into swiftlang:tensorflow May 16, 2019
@rxwei rxwei deleted the no-cotan branch May 16, 2019 10:53
@dan-zheng
Copy link
Contributor

How about alternative names for TangentVector, like Derivative?

I feelTangentVector may be more mathematically correct and obscure, while names like Derivative are less correct but more familiar.

cc @marcrasi

@marcrasi
Copy link

I like the idea of changing it to something more familiar. Ideas:

  • DerivativeVector (I think Derivative is too easy to confuse with the actual differential/pullback/gradient functions, but sticking Vector after it clarifies that it's a single value.)
  • GradientVector
  • ChangeVector
  • RateOfChangeVector
  • Velocity (This one is nice because it's really concrete and familiar. But it's only technically correct when you're taking a derivative of a path in space with respect to time. And it would be super confusing if you take a second derivative with respect to time, and get an acceleration value with type Velocity).

@saeta
Copy link
Contributor

saeta commented May 16, 2019

+1 to DerivativeVector :-)

rxwei pushed a commit to tensorflow/swift-apis that referenced this pull request May 16, 2019
This PR is parallel to the following [change](swiftlang/swift#24825) which deprecated `Cotangent` and removed the `tangentVector(from:`) method.
bartchr808 added a commit to bartchr808/swift that referenced this pull request May 16, 2019
rxwei added a commit to rxwei/swift that referenced this pull request Aug 25, 2019
`CotangentVector` was deprecated in swiftlang#24825 and the deprecation was released as part of Swift for TensorFlow v0.4. Now it's time to remove it.
rxwei added a commit that referenced this pull request Aug 25, 2019
`CotangentVector` was deprecated in #24825 and the deprecation was released as part of Swift for TensorFlow v0.4. Now it's time to remove it.
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.

4 participants