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

Minor tweaks to Differentiable protocol. #21521

Merged

Conversation

dan-zheng
Copy link
Contributor

@dan-zheng dan-zheng commented Dec 22, 2018

  • Conform TangentVector and CotangentVector to
    VectorNumeric where Scalar : FloatingPoint.
  • Rename moved(toward:) to moved(along:).

Also remove extraneous shape-related VectorNumeric requirements from
FloatingPoint types. Those requirements are no longer necessary now
that ShapedVectorNumeric has been split from VectorNumeric.

@dan-zheng dan-zheng requested a review from rxwei December 22, 2018 10:50
@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow

Copy link
Contributor

@rxwei rxwei left a comment

Choose a reason for hiding this comment

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

Nice!

@@ -60,23 +60,25 @@ public extension VectorNumeric {
public protocol Differentiable {
/// The tangent vector space of this differentiable manifold.
associatedtype TangentVector : Differentiable
where TangentVector.TangentVector == TangentVector
where TangentVector.TangentVector == TangentVector,
TangentVector.Scalar : FloatingPoint
Copy link
Contributor

Choose a reason for hiding this comment

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

Regarding the build error: I think you need to specify TangentVector's conformance to VectorNumeric here.

@dan-zheng dan-zheng force-pushed the differentiable-protocol-tweaks branch from db2574f to 5b29e9b Compare December 22, 2018 11:11
@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow

@rxwei
Copy link
Contributor

rxwei commented Dec 22, 2018

Ah right, tests should be updated

@dan-zheng
Copy link
Contributor Author

dan-zheng commented Dec 22, 2018

Updating tests soon!

Since the TangentVector and CotangentVector associated types must now conform to VectorNumeric, Differentiable types where TangentVector = CotangentVector = Self effectively must conform to VectorNumeric too.

I suppose this is fine and intentional. Tensor where Scalar : Numeric and FloatingPoint all conform to VectorNumeric. Derived conformances will eventually synthesize requirements for both VectorNumeric and Differentiable.

@dan-zheng dan-zheng force-pushed the differentiable-protocol-tweaks branch 2 times, most recently from 175a0b3 to 71bb3e1 Compare December 22, 2018 19:08
- Conform `TangentVector` and `CotangentVector` to
  `VectorNumeric where Scalar : FloatingPoint`.
- Rename `moved(toward:)` to `moved(along:)`.

Also remove extraneous shape-related `VectorNumeric` requirements from
`FloatingPoint` types. Those requirements are no longer necessary now
that `ShapedVectorNumeric` has been split from `VectorNumeric`.
@dan-zheng dan-zheng force-pushed the differentiable-protocol-tweaks branch from 71bb3e1 to 3cb371b Compare December 22, 2018 19:41
@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow

@rxwei rxwei merged commit 4d4f4b2 into swiftlang:tensorflow Dec 22, 2018
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.

2 participants