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

Make a better UX around the issue of tensorflow ABI instability. #1317

Closed
gabrieldemarmiesse opened this issue Mar 16, 2020 · 5 comments · Fixed by #1562
Closed

Make a better UX around the issue of tensorflow ABI instability. #1317

gabrieldemarmiesse opened this issue Mar 16, 2020 · 5 comments · Fixed by #1562

Comments

@gabrieldemarmiesse
Copy link
Member

Describe the feature and the current behavior/state.

See #1298

Currently, if we compile Tensorflow Addons against tensorflow 2.1.0, we have no garantees that it's going to work with another version of tensorflow. We don't even have a garantee that it will work with the same version if tensorflow has been compiled from source. As such, we should try to find ways of making this as painless as possible to users until Tensorflow provides backward compatibily garantees about the ABI (see tensorflow/community#77, tensorflow/community#101 and tensorflow/community#133). Ideas welcome.

@failure-to-thrive
Copy link
Contributor

Make a [black]list of builds/hashes of .so/.dllthat are seems to be good / proven to be bad. Print out appropriate warnings.

@MarkDaoust
Copy link
Member

If you have a different version installed, tfa-nightly does warn that it was built for TensorFlow 2.1 and may not work. So steps have already been taken towards @failure-to-thrive's suggestion. It's just not included in the 0.8 package.

@gabrieldemarmiesse
Copy link
Member Author

gabrieldemarmiesse commented Mar 16, 2020

It's just not included in the 0.8 package.

#1307 was merged. We just need to make a new release. @seanpmorgan , you may want to add this to your todo list.

I propose that we make a 0.9 release, compiled against TF 2.1, and at the same time, we make a 0.10 release, compile against TF 2.2, both with an appropriate warning message similar to the one we have currently. We'll need to backport all the bugfixes twice, but with the bot in place it's fairly painless.

@gabrieldemarmiesse
Copy link
Member Author

I believe we also need a section in our readme describing the ins and outs of the problem so that users really understand what is going on and why our compatibility with TF is so uncommon. We can put a link to this section in our warning message.

@gabrieldemarmiesse
Copy link
Member Author

gabrieldemarmiesse commented Mar 17, 2020

We have two types of incompatibilities:

  1. Python incompatibilities
  2. ABI incompatibility.

It's possible for a user to use the python-only features and be completely fine. So I think we should have two warning messages, one at import time, for python incompatibility, and one when loading the first SO, to warn about ABI incompatibility. We except more compatibility in the python world than in the ABI world so the conditions for printing the warning should be different.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants