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

[feat] compatible with TF1.15.2 #90

Merged
merged 1 commit into from
Jun 3, 2021

Conversation

rhdong
Copy link
Member

@rhdong rhdong commented Jun 2, 2021

Description

Brief Description of the PR:

  • compatible with TF1.15.2
  • add CI docker files
  • Add TF1152 to workflow
  • change bazel version to 3.1.0
  • remove TF from requirement.txt and make it setup by code
  • Update README.md
  • change GPU compute Capability to 60/70/75/80

Type of change

  • Bug fix
  • New Tutorial
  • Updated or additional documentation
  • Additional Testing
  • New Feature

Checklist:

  • I've properly formatted my code according to the guidelines
    • By running Yapf and clang-format
    • By running pre-commit hooks
  • This PR addresses an already submitted issue for TensorFlow Recommenders Addons
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • This PR contains modifications to C++ custom-ops

How Has This Been Tested?

If you're adding a bugfix or new feature please describe the tests that you ran to verify your changes:

  • Add TFRA with TF1152-CPU to CI workflow
  • Test TFRA with TF1152-GPU manually on Nvidia T4, V100 and P40

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
RUN rm /usr/bin/gcc
ENV PATH=/dt7/usr/bin${PATH:+:${PATH}}
# Use devtoolset-7 as tool chain
RUN rm -r /usr/bin/gcc*
Copy link
Member Author

Choose a reason for hiding this comment

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

@zhangyafeikimi Pls help check it, I have force remove the gcc* in /usr/bin/ and use devtoolset-7.

Copy link
Contributor

Choose a reason for hiding this comment

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

not make sense

tools/docker/devtoolset/build_devtoolset.sh Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@rhdong rhdong added the enhancement New feature or request label Jun 2, 2021
@rhdong rhdong linked an issue Jun 2, 2021 that may be closed by this pull request
@rhdong rhdong force-pushed the rhdong/compatible-with-tf1152 branch 4 times, most recently from e8a5bfa to 7def57c Compare June 2, 2021 11:40
@@ -56,6 +56,8 @@ TF_CALL_int8(REGISTER_KERNELS);
TF_CALL_int32(REGISTER_KERNELS);
TF_CALL_half(REGISTER_KERNELS);
TF_CALL_float(REGISTER_KERNELS);
TF_CALL_tstring(REGISTER_KERNELS);
Copy link
Member

@Lifann Lifann Jun 3, 2021

Choose a reason for hiding this comment

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

It seems that the corresponding functor has not been defined to support tstring.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, I removed it.

@rhdong rhdong force-pushed the rhdong/compatible-with-tf1152 branch from 651a160 to 52eef53 Compare June 3, 2021 03:13
@Lifann
Copy link
Member

Lifann commented Jun 3, 2021

Please also decorate the names here and here

@rhdong
Copy link
Member Author

rhdong commented Jun 3, 2021

Please also decorate the names here and here

No need for them, the decorate macro is created for history reason, we had to compatible with TF1.x naming rule which doesn't support the "XYZ>AbcEfg" rule, but we cann't change the name directly for some user already use TFRA release 1 in product environment, and change name to "TfraXyz" may cause compatible problem. For new OPs, we will not use ">" at first.

@rhdong rhdong force-pushed the rhdong/compatible-with-tf1152 branch 2 times, most recently from a9e8858 to 748aab9 Compare June 3, 2021 06:24
- compatible with TF1.15.2
- add CI docker files
- Add TF1152 to workflow
- change bazel version to 3.1.0
- remove TF from requirement.txt and make it setup by code
- Update README.md
- change GPU compute Capability to 60/70/75/80
@rhdong rhdong force-pushed the rhdong/compatible-with-tf1152 branch from 748aab9 to fe8538a Compare June 3, 2021 06:30
Copy link
Member

@Lifann Lifann left a comment

Choose a reason for hiding this comment

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

LGTM

@rhdong rhdong merged commit 63899a4 into tensorflow:master Jun 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is there a TFRA package that is compatible with TensorFlow 1.x?
4 participants