Skip to content
This repository has been archived by the owner on Jul 1, 2023. It is now read-only.

Remove ParameterlessLayer conformance workarounds #1037

Open
texasmichelle opened this issue Jul 3, 2020 · 1 comment
Open

Remove ParameterlessLayer conformance workarounds #1037

texasmichelle opened this issue Jul 3, 2020 · 1 comment
Assignees

Comments

@texasmichelle
Copy link
Member

Associated type inference behavior was changed in apple/swift#32578:
derived conformances are now attempted before associated type inference.

This broke ParameterlessLayer, which relied on a
TangentVector == EmptyTangentVector same-type constraint to set a
default TangentVector type witness for conforming types.

A workaround was added in several PRs (#1034, #1035, swift-models#620, fastai/fastai_dev) and is forward-and backward-compatible, but makes code more verbose. If possible, identify a way to support the old behavior.

This caused build errors in the 2020-06-30 master -> tensorflow merge:

tensorflow-swift-apis/Sources/TensorFlow/Layers/Core.swift:21:15: error: 'ParameterlessLayer' requires the types 'Flatten<Scalar>.TangentVector' and 'EmptyTangentVector' be equivalent
public struct Flatten<Scalar: TensorFlowFloatingPoint>: ParameterlessLayer {
              ^
tensorflow-swift-apis/Sources/TensorFlow/Layers/Core.swift:21:15: note: requirement specified as 'Self.TangentVector' == 'EmptyTangentVector' [with Self = Flatten<Scalar>]
public struct Flatten<Scalar: TensorFlowFloatingPoint>: ParameterlessLayer {
              ^
tensorflow-swift-apis/Sources/TensorFlow/Layers/Core.swift:21:15: error: type 'Flatten<Scalar>.TangentVector' does not conform to protocol 'VectorProtocol'
public struct Flatten<Scalar: TensorFlowFloatingPoint>: ParameterlessLayer {
              ^
tensorflow-swift-apis/Sources/TensorFlow/Layers/Core.swift:39:15: error: 'ParameterlessLayer' requires the types 'Reshape<Scalar>.TangentVector' and 'EmptyTangentVector' be equivalent
public struct Reshape<Scalar: TensorFlowFloatingPoint>: ParameterlessLayer {
              ^
tensorflow-swift-apis/Sources/TensorFlow/Layers/Core.swift:39:15: note: requirement specified as 'Self.TangentVector' == 'EmptyTangentVector' [with Self = Reshape<Scalar>]
public struct Reshape<Scalar: TensorFlowFloatingPoint>: ParameterlessLayer {
              ^
tensorflow-swift-apis/Sources/TensorFlow/Layers/Core.swift:72:15: error: 'ParameterlessLayer' requires the types 'Function<Input, Output>.TangentVector' and 'EmptyTangentVector' be equivalent
public struct Function<Input: Differentiable, Output: Differentiable>: ParameterlessLayer {
              ^
tensorflow-swift-apis/Sources/TensorFlow/Layers/Core.swift:72:15: note: requirement specified as 'Self.TangentVector' == 'EmptyTangentVector' [with Self = Function<Input, Output>]
public struct Function<Input: Differentiable, Output: Differentiable>: ParameterlessLayer {
              ^
tensorflow-swift-apis/Sources/TensorFlow/Layers/Core.swift:72:15: error: type 'Function<Input, Output>.TangentVector' does not conform to protocol 'VectorProtocol'
public struct Function<Input: Differentiable, Output: Differentiable>: ParameterlessLayer {
              ^
saeta pushed a commit to fastai/fastai_dev that referenced this issue Jul 3, 2020
This week, the S4TF team encountered several failures building toolchains:

1. Outdated Package.resolved in [swift/FastaiNotebook_11_imagenette](https://github.com/fastai/fastai_dev/blob/master/swift/FastaiNotebook_11_imagenette/Package.resolved).
2. A regression in `ParameterlessLayer` caused by a change in type inference behavior in [swiftlang/swift#32578](swiftlang/swift#32578).

This PR includes two workarounds that unblock toolchain builds:
1. Remove Package.resolved ([swift-apis/1036](tensorflow/swift-apis#1036)). Since this file is regenerated on build, it's often not necessary to include in source control.
2. Add a typealias to instances conforming to `ParameterlessLayer`, which was effective in swift-apis and swift-models (see [tensorflow/swift-apis#1037](tensorflow/swift-apis#1037)).

I'm happy to create an additional PR to remove the other Package.resolved files in this repo and add to .gitignore, if desired.
@dan-zheng
Copy link
Member

@marcrasi: I wonder if this is effectively fixed by your recent TangentVector derived conformances changes?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants