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 2 compatibility #165

Open
githubpiyush opened this issue Feb 3, 2020 · 17 comments
Open

Tensorflow 2 compatibility #165

githubpiyush opened this issue Feb 3, 2020 · 17 comments

Comments

@githubpiyush
Copy link

Can you tell me when can we expect this package with tensorflow 2.1.0 ?

@githubpiyush githubpiyush changed the title Tensorflow 2 package Tensorflow 2 compatibility Feb 3, 2020
@githubpiyush
Copy link
Author

@emedvedev Any updates??

@OctaM
Copy link

OctaM commented Feb 17, 2020

Or maybe just a guideline on what we need to change in order to work with tf2

@githubpiyush
Copy link
Author

@OctaM Yes that would be really helpful.
By the way i have converted most of the part using tf compat v1 and I have stuck when there is hash table defined in model.py file under model directory because dtype of (key, value) as (int, string) has been removed.

@vladdders
Copy link

vladdders commented Feb 28, 2020

I performed the same steps and got stuck there as well. The main issue seems to be the fact that the project is very dependent on the contrib module which has been completely removed in v2 and I can't seem to find many of the equivalent operations in other repos...

@emedvedev
Copy link
Owner

There's no plan to move this project to Tensorflow 2 yet, unfortunately, but contributions are welcome. I could create a branch to submit PRs against if there are several people who'd like to collaborate on that.

@khu834
Copy link

khu834 commented Feb 23, 2021

@emedvedev
I'd be happy to contribute to a TF2 conversion. A branch for folks to merge into would be very useful.

@emedvedev
Copy link
Owner

@khu834 thank you so much! Created a https://github.com/emedvedev/attention-ocr/tree/tf2 branch :)

@khu834
Copy link

khu834 commented Feb 23, 2021

Awesome, by the way, this is to completely convert to TF2 right? Not using tensorflow.compat.v1
Let me know and I'll take a stab at it.

@khu834
Copy link

khu834 commented Feb 23, 2021

@OctaM Yes that would be really helpful.
By the way i have converted most of the part using tf compat v1 and I have stuck when there is hash table defined in model.py file under model directory because dtype of (key, value) as (int, string) has been removed.

Have you tried tf.raw_ops.MutableHashTable?
https://www.tensorflow.org/api_docs/python/tf/raw_ops/MutableHashTable

The interface seems like it should do the same as tf.contrib.lookup.MutableHashTable

@khu834
Copy link

khu834 commented Feb 23, 2021

I performed the same steps and got stuck there as well. The main issue seems to be the fact that the project is very dependent on the contrib module which has been completely removed in v2 and I can't seem to find many of the equivalent operations in other repos...

Most instances of contrib usage I found are fairly straightforward to replace. Things like xavier_initializer, batch_norm, dropout, and all the RNN cell types should all have TF2 equivalents.

Which ones haven't you found a good replacement for?

@emedvedev
Copy link
Owner

emedvedev commented Feb 23, 2021 via email

@khu834
Copy link

khu834 commented Feb 24, 2021

@emedvedev
I've read through the code, doing some planning now in terms of the code structuring.
Overall, would you mind if we convert to tf.keras API where possible? In TF2 I think that's pretty much the preferred interface at least for the model definition.
CNN model I think definitely makes sense to write in tf.keras. This also allows substituting the feature extraction with other models such as ResNet easily.
Seq2Seq model I think should be convertible to tf.keras. I've done seq2seq before in keras it's not too bad.

The training loop is not as straightforward, but may still be doable with tf.keras training routine (combined with a few custom Callback), this may take a bit more experimentation. Maybe initially we can keep this in tensorflow ops using GradientTape.

Let me know if you have preferences

@emedvedev
Copy link
Owner

emedvedev commented Feb 24, 2021 via email

@micahprice
Copy link

It would be great in the meantime to open and pr to a branch intended to work using tensorflow.compat.v1 and fixing tf.contrib dependencies. Or @githubpiyush and @vladdders, was that more difficult than expected?

@githubpiyush
Copy link
Author

It would be great in the meantime to open and pr to a branch intended to work using tensorflow.compat.v1 and fixing tf.contrib dependencies. Or @githubpiyush and @vladdders, was that more difficult than expected?

Ah! It's more than a year now and from what i remember i have completed this with tf.compat.v1 and yes it was a little difficult 😢 I don't have the code right now 😶

@mjpieters
Copy link
Contributor

I've created a PR with the minimal work required to make the project functional on TF2: #198

@emedvedev
Copy link
Owner

Merged @mjpieters's PR: #198. Should support TF2 now.

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

No branches or pull requests

7 participants