Skip to content

[TF] WIP: Removing all instances of #tfop. #24425

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

Merged
merged 11 commits into from
May 2, 2019
Merged

[TF] WIP: Removing all instances of #tfop. #24425

merged 11 commits into from
May 2, 2019

Conversation

eaplatanios
Copy link

@rxwei @pschuh @saeta This is a WIP PR to remove all instances of #tfop. It's WIP because I haven't yet run tests locally for these changes (currently compiling from scratch so it may take a while). Also, I assumed the uses of #tfop("tfc.configureGPU") etc also deprecated and went ahead and removed them.

I'll wait for the tests to run and see if anything breaks from the removed functionality that was not already disabled by @pschuh's PRs.

@pschuh
Copy link
Contributor

pschuh commented May 2, 2019

This is missing the utils/build-script-impl changes and utils/build_swift/driver_arguments.py

ie:

 # Copy TensorFlow bindings file, if it exists.
 if (TENSORFLOW_SWIFT_BINDINGS)
-  list(APPEND SOURCES "${TENSORFLOW_SWIFT_BINDINGS}")
+  file(GLOB_RECURSE TENSORFLOW_SWIFT_BINDINGS_SOURCES
+    "${TENSORFLOW_SWIFT_BINDINGS}/*.swift")
+  list(APPEND SOURCES "${TENSORFLOW_SWIFT_BINDINGS_SOURCES}")
 endif()

and

-                        TENSORFLOW_SWIFT_BINDINGS="${TENSORFLOW_SWIFT_BINDINGS_DIR}/RawOpsGenerated.swift"
+                        TENSORFLOW_SWIFT_BINDINGS="${TENSORFLOW_SWIFT_BINDINGS_DIR}"

@eaplatanios
Copy link
Author

eaplatanios commented May 2, 2019

@pschuh The only tests that currently fail are:

  • TensorFlowRuntime/model_autodiff_runtime.swift
  • TensorFlowRuntime/tensor_autodiff_indirect.swift
  • TensorFlowRuntime/tensor_autodiff_runtime.swift

but I'm not sure why they fail. I also had to add // REQUIRES: deprecated_gpe_mode to deabstraction_crashers.swift and dynamic_attributes.swift.

@eaplatanios
Copy link
Author

@pschuh Nevermind the above problem. I fixed that. I'm currently removing tests using .toHost and .toAccelerator since they're not relevant anymore.

@eaplatanios
Copy link
Author

@pschuh @rxwei All tests pass on my machine now.

@pschuh
Copy link
Contributor

pschuh commented May 2, 2019

Update the hash, and then we can do the swift-ci for this PR.

@eaplatanios
Copy link
Author

I just updated it.

@pschuh
Copy link
Contributor

pschuh commented May 2, 2019

@swift-ci please test tensorflow clean

@eaplatanios
Copy link
Author

Not sure if the tests are actually running as nothing is showing up on GitHub for me.

@pschuh
Copy link
Contributor

pschuh commented May 2, 2019

@swift-ci please test tensorflow clean

@pschuh
Copy link
Contributor

pschuh commented May 2, 2019

@swift-ci please test tensorflow

@pschuh
Copy link
Contributor

pschuh commented May 2, 2019

@swift-ci please test tensorflow clean

@pschuh
Copy link
Contributor

pschuh commented May 2, 2019

@swift-ci please clean test tensorflow

@pschuh pschuh merged commit b8a41ae into swiftlang:tensorflow May 2, 2019
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