-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[AutoDiff][IRGen] Can't compile Swift for TensorFlow on top-of-tree Swift #59467
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
Comments
I found the reproducer while testing an old branch that was usable only months ago. It was pretty much the same as Fan's branch, which was only usable with the November 12, 2021 toolchain. But I just tested top-of-tree S4TF at s4tf/s4tf, and the crash appears there as well. I'm surprised it slipped under my nose for almost 30 days. I had been testing only the 5.7 branch snapshots since exactly May 18. I had both the trunk toolchain and 5.7 branch toolchain for May 18 on my computer, and thought the trunk toolchain was running. Apparently it was the 5.7 branch toolchain. |
I am unable to reproduce the crash on top-of-tree as a Stdlib regression test. I tried replication the style of @asl's The base Swift repository doesn't include Here is the progress I made. Add this file to import _Differentiation
struct Tensor: Differentiable {}
// `Tensor` could be defined in this test case's primary file and the crash
// would still happen. All that matters is that `LayerNorm_callAsFunction` and
// `rsqrt` are defined in separate files.
@differentiable(reverse)
func rsqrt( _ x: Tensor) -> Tensor {
fatalError()
}
@derivative(of: rsqrt)
func _vjpRsqrt(_ x: Tensor) -> (
value: Tensor, pullback: (Tensor.TangentVector) -> (Tensor.TangentVector)
) {
fatalError()
} And add the following as // RUN: %target-swift-frontend -emit-ir -primary-file %s %S/Inputs/59467-???.swift -module-name main -o /dev/null
import _Differentiation
@_semantics("autodiff.nonvarying")
func withoutDerivative() -> Tensor {
fatalError()
}
func BatchNorm_doInference(
_ input: Tensor
) -> Tensor {
withoutDerivative()
}
@differentiable(reverse)
func BatchNorm_callAsFunction(_ input: Tensor) -> Tensor {
BatchNorm_doInference(input)
}
@differentiable(reverse)
func LayerNorm_callAsFunction(_ input: Tensor) -> Tensor {
rsqrt(input)
} Would you be able to finish what I started here? |
Can't you simply build a toolchain from branch and build your code with it? |
The closest I have to building a toolchain from scratch with my current skillset is:
I don't know how to compile a full-on toolchain that includes SwiftPM and everything added on to the bare apple/swift repo. I was hoping that you could do so more easily than I could. |
Hopefully this: https://github.com/apple/swift-package-manager/blob/main/CONTRIBUTING.md will help. Check |
I'm still struggling to get that working. Overall, I think it's more time-effective if you test this bug with SwiftPM. Plus, it was PassiveLogic that originally encountered the bug (#59467 (comment)) without doing the necessary work to fully narrow down and report it. I don't mean to be disrespectful, but I have been narrowing this bug for ~12 hours straight and I think it logically makes sense that you and Brad share some of the responsibility of investigating this bug. |
I'll let @BradLarson and PassiveLogic folks take care about test reduction for you. |
I don't have a standalone lit test, but I can say that a SwiftPM package with the original configuration described above does reproduce this error on multiple platforms with toolchains since the original derivative registration fix went in. A toolchain with #58965 applied builds this package without error. That may indeed just be a suppression of the underlying problem, but the error does not reproduce after that patch. The swiftc invocation that triggers this for that package goes something like the following:
When we saw this, it was highly dependent on specific locations and names of functions. A workaround was to simply rename or move the file that was triggering the error, and the rest would build fine. I didn't pursue it further when I saw that #58965 prevented it in our local cases. |
Thanks! I will be fine if #58965 is destined to be merged before the release of Swift 5.8. This means a deadline of March 2023 - months into the future. Could we add this bug into I still need to test your workaround of renaming files and relocating code. Regardless, the bug impacts my project and I would not be happy if it silently reappeared. Adding this crash to regression tests as part of #58965 would instantly notify us if it reappears. |
I reproduced the crash without SwiftPM! I should be able to carry on from here and author the regression test for #58965. Crash log
Build command narrowed down to:
Note: in the regression tests, |
* Add S4TF crasher test * Unbreak the test Co-authored-by: Philip Turner <[email protected]> Resolves #59467
I just tested S4TF against the latest trunk development snapshot, and it works! July 20, 2022 Swift.org trunk development snapshot, Google Colaboratory (Ubuntu 18.04, x86_64). All of the S4TF tutorials made by the TensorFlow organization work as well. |
* Add S4TF crasher test * Unbreak the test Co-authored-by: Philip Turner <[email protected]> Resolves swiftlang#59467
Describe the bug
This bug appeared between the May 11, 2022 and May 18, 2022 Trunk Development Snapshots. It seems to be caused by changing behavior of cross-file lookup for derivatives, resulting from the merging of #58644. It seems similar to #55170. The suspected source of this bug was not merged into the
release/5.7
branch, so it is not a great concern for now. I will still be able to compile on the Swift 5.7 release toolchain, which I am currently targeting.To Reproduce
Steps to reproduce the behavior:
Package.swift
:Sources/TensorFlow/Core/Tensor.swift
:Sources/TensorFlow/Core/Layers/Normalization.swift
Crash stack trace
Environment (please complete the following information):
The text was updated successfully, but these errors were encountered: