emedvedev / attention-ocr

A Tensorflow model for text recognition (CNN + seq2seq with visual attention) available as a Python package and compatible with Google Cloud ML Engine.
MIT License
1.08k stars 256 forks source link

Tensorflow 2 compatibility #165

Open githubpiyush opened 4 years ago

githubpiyush commented 4 years ago

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

githubpiyush commented 4 years ago

@emedvedev Any updates??

OctaM commented 4 years ago

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

githubpiyush commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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 commented 3 years ago

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

emedvedev commented 3 years ago

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

khu834 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

Ideally to completely convert, yes :) On Feb 23, 2021, 12:41 +0700, Kevin Chih Yao Huang notifications@github.com, wrote:

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? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

khu834 commented 3 years ago

@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 commented 3 years ago

I don't mind, tf.keras would make lots of sense imo. Thanks a lot for doing this! On Feb 25, 2021, 03:14 +0700, Kevin Chih Yao Huang notifications@github.com, wrote:

@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 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 Callbacks), 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 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

micahprice commented 3 years ago

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 commented 3 years ago

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 commented 1 year ago

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

emedvedev commented 1 year ago

Merged @mjpieters's PR: https://github.com/emedvedev/attention-ocr/pull/198. Should support TF2 now.