Skip to content
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

[tensorflow] Add support for the is_symbolic_tensor predicate #22878

Merged
merged 1 commit into from
Apr 20, 2023

Conversation

hvaara
Copy link
Contributor

@hvaara hvaara commented Apr 20, 2023

What does this PR do?

This PR adds support for the is_symbolic_tensor predicate in TensorFlow. This predicate will become available starting with version 2.14.

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Apr 20, 2023

The documentation is not available anymore as the PR was closed or merged.

This predicate will become available in tensorflow starting with version
2.14.

Co-authored-by: Russell Power <power@google.com>
@hvaara hvaara marked this pull request as ready for review April 20, 2023 03:06
@amyeroberts
Copy link
Collaborator

cc @Rocketknight1

@Rocketknight1
Copy link
Member

This looks clean! Do you have any links to documentation/discussion about is_symbolic_tensor in TF, though? This is the first I've heard of it - I can see the relevant code in the TF codebase, but are we guaranteed that the behaviour won't change before the 2.14 release? Also, given that it's in the codebase already, won't it be in the 2.13 release rather than 2.14?

@rjpower
Copy link
Contributor

rjpower commented Apr 20, 2023

I wrote the is_symbolic_tensor code in question. The 2.14 was a guess from me, I wasn't sure if the original PR was going to make the TF branch cut, but it looks like it will make 2.13. The intention of is_symbolic_tensor is actually to provide more stability:

We're looking into breaking the current inheritance setup in TF. EagerTensor inherits from symbolic Tensor, and this adds a lot of weird complication in the TF codebase, along with awkward checks like type(t) == Tensor. This method was introduced to avoid churning the few users who need to distinguish between eager and symbolic tensors.

We don't have a proposal yet for the split, but we're front-running some of this prep/cleanup.

@Rocketknight1
Copy link
Member

LGTM in that case, and thanks for the clarification!

@Rocketknight1 Rocketknight1 merged commit 515d6a5 into huggingface:main Apr 20, 2023
4 checks passed
@hvaara hvaara deleted the tf-symbolic-tensor branch April 22, 2023 03:55
novice03 pushed a commit to novice03/transformers that referenced this pull request Jun 23, 2023
…ingface#22878)

This predicate will become available in tensorflow starting with version
2.14.

Co-authored-by: Russell Power <power@google.com>
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.

None yet

5 participants