-
Notifications
You must be signed in to change notification settings - Fork 135
NFC: fix warnings related to VectorProtocol.VectorSpaceScalar
.
#1178
base: main
Are you sure you want to change the base?
NFC: fix warnings related to VectorProtocol.VectorSpaceScalar
.
#1178
Conversation
`VectorProtocol.VectorSpaceScalar` was changed from an associated type to a hardcoded typealias for `Float`. Fix warnings related to this change.
@@ -33,8 +33,7 @@ import Numerics | |||
public class RMSProp<Model: Differentiable>: Optimizer | |||
where | |||
Model.TangentVector: VectorProtocol & PointwiseMultiplicative | |||
& ElementaryFunctions & KeyPathIterable, | |||
Model.TangentVector.VectorSpaceScalar == Float | |||
& ElementaryFunctions & KeyPathIterable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nifty Vim search-and-replace command: :%s/,\_s*Model.TangentVector.VectorSpaceScalar == Float//g
.
Apparently the \_s
pattern matches a whitespace, tab, or newline character (source).
@@ -47,8 +47,7 @@ import _Differentiation | |||
/// ```` | |||
public struct Sequential<Layer1: Module, Layer2: Layer>: Module |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kokoro CI fails (for TENSORFLOW_USE_STANDARD_TOOLCHAIN=NO
):
/swift-apis/Sources/TensorFlow/Layers/Sequential.swift:68:1: error: type 'Sequential<Layer1, Layer2>.TangentVector' does not conform to protocol 'VectorProtocol'
}
^
Swift.VectorProtocol:2:20: note: protocol requires nested type 'VectorSpaceScalar'; do you want to add it?
associatedtype VectorSpaceScalar : AdditiveArithmetic
^
It may be possible to fix this by adding an explicit typealias VectorSpaceScalar = Float
definition for Sequential.TangentVector
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried doing this in 34fca6a but it didn't work, so I'm not sure what to try next.
Does anyone else have ideas? @BradLarson @compnerd
One option is to bite the bullet and drop support for the old baked-in-TensorFlow
toolchains (which is what's causing the failure). I think that's a good option if we and the community are all on-board with it. That also allows us to drop custom internal support for CI and building OSS toolchains - we can just use GitHub Actions, which is much simpler.
Fix type-checking error stating that `Sequential.TangentVector` does not satisfy `VectorProtocol.VectorSpaceScalar` requirement.
727c600
to
34fca6a
Compare
VectorProtocol.VectorSpaceScalar
was changed from an associated type to a hardcoded typealias forFloat
.(I believe this simplification (removing
VectorSpaceScalar
as a customization point) made it easier to enable buildingtensorflow/swift-apis
with standard toolchains.)Fix warnings related to this change.
Example warnings: