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

TensorFlow: annotate functions using _forEachFieldWithKeyPath #1162

Merged
merged 1 commit into from
Dec 21, 2020

Conversation

compnerd
Copy link
Contributor

Because the standard toolchain support requires the newest runtime in
order to have access to _forEachFieldWithKeyPath and the runtime is
bundled into the OS on macOS, we need to annotate the functions with
availability.

Copy link
Member

@dan-zheng dan-zheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this solves usability of the attributed declarations on macOS in practice? I fear end users will still encounter an availability error. We could verify this by adding a test case.

I'm not sure how to fix this in a robust way? May be good to ask the forums.

Because the standard toolchain support requires the newest runtime in
order to have access to `_forEachFieldWithKeyPath` and the runtime is
bundled into the OS on macOS, we need to annotate the functions with
availability.
@compnerd
Copy link
Contributor Author

@dan-zheng it didn't - we needed to restrict this to a runtime check, which I have now done. This was sufficient to get this to build the package with the latest snapshot locally.

@compnerd compnerd requested a review from pschuh December 21, 2020 21:56
@compnerd compnerd merged commit 8a910b4 into tensorflow:main Dec 21, 2020
@compnerd compnerd deleted the annotation branch December 21, 2020 23:26
@philipturner
Copy link

I fear end users will still encounter an availability error.

Reverted by s4tf#17. This causes runtime crashes and no longer serves a functional purpose. On release toolchains, S4TF uses swift-reflection-mirror for reflection. I currently have a bug in ReflectionMirror exposed by comparing arrays of keypaths, but that should be fixable.

@philipturner
Copy link

Nevermind. I have to add these guards back in to make S4TF compile on development toolchains. But they will be gated under #if !TENSORFLOW_USE_RELEASE_TOOLCHAIN.

philipturner added a commit to philipturner/s4tf that referenced this pull request Jul 4, 2022
philipturner added a commit to s4tf/s4tf that referenced this pull request Jul 6, 2022
* Initial support for release toolchains

* Fix typo

* Remove fatal error

* Comment out x10_training_loop for Colab

* Make `Mergeable.stack` differentiable, working around swiftlang/swift#59876

* Remove #available(macOS 9999, *) restriction

* Enable RNN tests

* Re-enable tensorflow#1162

* Allow Array replaceSubrange

* Fit within 100 spaces

* Type fix "partecipating" -> "participating"

* Remove redundant same-type constraint

* Remove deprecated Dataset API and reposition Zip2TensorGroup

* Refactor tests

* Refactor X10 tests

* Fix failing tests on macOS + arm64

* Remove workarounds for swiftlang/swift#55703 (SR-13263)

* Enable ops_test.swift

* Optimize ops_test.swift

* Optimize more tests

* Rename gradient(at:in) to gradient(at:of:)

* Rename argument label in withoutDerivative

* Enable tests blocked by reflection crash

* Workaround for swiftlang/swift#59135 on dev toolchains

* Fix tests for Colab

* Attempt to speed up tests on Colab

* Revert changes to tests

* Fix tests on Colab NVIDIA GPU

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

Successfully merging this pull request may close these issues.

4 participants