Skip to content

Conversation

@eaplatanios
Copy link

@eaplatanios eaplatanios commented May 30, 2019

@rxwei @pschuh Removed HackyTensorflowMigrationSupport.swift.

Friend PR: tensorflow/swift-apis#139.

@rxwei rxwei added the tensorflow This is for "tensorflow" branch PRs. label May 30, 2019
rxwei pushed a commit to tensorflow/swift-apis that referenced this pull request May 30, 2019
Removed HackyTensorflowMigrationSupport.swift from apple/swift.

Friend PR: swiftlang/swift#25147.
@eaplatanios
Copy link
Author

@rxwei I update the dependency hash so we should be able to run tests now.

@rxwei
Copy link
Contributor

rxwei commented May 30, 2019

@swift-ci please clean test tensorflow Linux

@eaplatanios
Copy link
Author

eaplatanios commented May 30, 2019

@rxwei, this is a separate issue but for lack of a better communication channel I'll post it here. I tried enabling compiler optimizations for the TensorFlow module by setting these flags:

list(APPEND swift_stdlib_compile_flags "-O" "-whole-module-optimization")

but I still get failures for multiple tests even though GPE is now gone. I believe we should start looking into why this happens as it could potentially have significant performance impact. One very simple test that fails with a set fault, for example, is this:

StringDescriptionTests.test("Vector") {
  do {
    let vector = Tensor<Int32>(ones: [4])
    expectEqual("[1, 1, 1, 1]", vector.description)
  }

  do {
    var vector = Tensor<Float>([1, 2, 3, 4])
    expectEqual("[1.0, 2.0, 3.0, 4.0]", vector.description)
    vector[1] = Tensor<Float>(-2)
    expectEqual("[ 1.0, -2.0,  3.0,  4.0]", vector.description)
  }

  // Test long vector (above 1000 scalar threshold).
  do {
    let vector = Tensor<Float>(repeating: 3, shape: [1001])
    expectEqual("[3.0, 3.0, 3.0, ..., 3.0, 3.0, 3.0]", vector.description)
  }
}

Again, this is separate from this PR so sorry for posting it here if it's confusing.

@bgogul
Copy link
Contributor

bgogul commented May 30, 2019

@rxwei @eaplatanios Is it OK to have known protocols and derived conformances for those protocols that are not part of stdlib?

@eaplatanios
Copy link
Author

@bgogul I believe that technically they are since TensorFlow is part of stdlib. They are just hosted in a separate repository.

Generally though, it'd be great if we found an elegant way to add support for such derivations at the language level, similar to what Scala supports, so that all this can be implemented in Swift. :)

@eaplatanios
Copy link
Author

I just pushed an edit that fixes the failing test. It only fails on Linux and I don't really understand why but it should be fine now.

@rxwei
Copy link
Contributor

rxwei commented May 30, 2019

@swift-ci please test tensorflow Linux

@rxwei
Copy link
Contributor

rxwei commented May 30, 2019

@swift-ci please test tensorflow

@rxwei rxwei merged commit 4762f67 into swiftlang:tensorflow May 30, 2019
rxwei pushed a commit to tensorflow/swift-apis that referenced this pull request May 30, 2019
Simplify `_ExecutionContext.init()` runtime initialization code.
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.

3 participants