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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated attention_ocr model to be compatible with TensorFlow 2.x. #10952

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

xavigibert
Copy link
Contributor

@xavigibert xavigibert commented Mar 9, 2023

Updated references to removed tf.contrib packages:

  • tf.contrib.slim is now distributed as a separate PIP package tf-slim.
  • tf.contrib.legacy_seq2seq is no longer available. The classes need by this model have been copied here.
  • tf.contrib.lookup.index_to_string_table_from_tensor has been replaced with tf.lookup.StaticHashTable.
  • tf.contrib.layers.one_hot_encoding has been replaced with tf.one_hot.

Added the FSNS list of URLs which used to be under research/street (that model was removed by PR #8934).

This update makes the research/attention_ocr model compatible with TensorFlow 2, but it is not TF2 native, since it uses compat.v1 layers. Checkpoints trained with TF1 can still be loaded with the latest version, but results my be slightly different due to implementation changes with respect to old contrib layers.

BUG: The test_moving_variables_properly_loaded_from_a_checkpoint in demo_inference.py no longer passes (probably to internal changes in SLIM). We need retrain the model and provide an updated checkpoint that makes this test pass.

Description

馃摑 Please include a summary of the change.

  • Please also include relevant motivation and context.
  • List any dependencies that are required for this change.

TensorFlow 1.15 is no longer supported, this model now works with TensorFlow 2.11. This change change introduces tf-slim as a new dependency (tf.contrib was removed in TF 2.0).

Type of change

For a new feature or function, please create an issue first to discuss it
with us before submitting a pull request.

Note: Please delete options that are not relevant.

  • Documentation update
  • TensorFlow 2 migration

Tests

馃摑 Please describe the tests that you ran to verify your changes.

  • Provide instructions so we can reproduce.
  • Please also list any relevant details for your test configuration.

Ran all the unit tests, using the procedure in https://github.com/tensorflow/models/tree/master/research/attention_ocr#how-to-use-this-code. All tests pass in Mac OS 10.15.7 and Debian Linux 10.

Also started training the model from scratch on the FSNS dataset and verified that it achieves 81% test performance after 24 hours. I will provide the checkpoint at 400k steps once it completes.

Test Configuration:

Cloud instance with 12 vCPUs, 48GB of RAM, 250GB persistent balanced storage, and 1 NVIDIA V100 GPU, running Google Deep Learning VM (version tf2-gpu.2-11.m103). Set up the environment as described in the README.md. Started 4 consoles using tmux and ran each of following commands in each console:

python train.py --max_number_of_steps=400000
python eval.py --split_name=train --eval_log_dir=/tmp/attention_ocr/eval_train
python eval.py --split_name=test --eval_log_dir=/tmp/attention_ocr/eval_test
python eval.py --split_name=validation --eval_log_dir=/tmp/attention_ocr/eval_validation

TensorBoard screenshot:

Screen Shot 2023-03-09 at 10 19 23 AM

Checklist

This code predates TensorFlow 2, so it does not meet the coding guidelines. There is an ongoing effort to migrate this model to Keras, but in order to facilitate this migration, we need to have this reference code still able to be run using the latest version of TensorFlow.

Updated references to removed `tf.contrib` packages:
- `tf.contrib.slim` is now distributed as a separate PIP package tf-slim.
- `tf.contrib.legacy_seq2seq` is no longer available. The classes need by this model have been copied here.
- `tf.contrib.lookup.index_to_string_table_from_tensor` has been replaced with `tf.lookup.StaticHashTable`.
- `tf.contrib.layers.one_hot_encoding` has been replaced with `tf.one_hot`.

Added the FSNS list of URLs which used to be under `research/street` (that model was removed by PR tensorflow#8934).
This update makes the `research/attention_ocr` model compatible with TensorFlow 2, but it is not TF2 native, since it uses compat.v1 layers. Checkpoints trained with TF1 can still be loaded with the latest version, but results my be slightly different due to implementation changes with respect to old contrib layers.

BUG: The `test_moving_variables_properly_loaded_from_a_checkpoint` in `demo_inference.py` no longer passes (probably to internal changes in SLIM). We need retrain the model and provide an updated checkpoint that makes this test pass.
@xavigibert xavigibert marked this pull request as draft March 10, 2023 10:09
@xavigibert xavigibert marked this pull request as ready for review March 10, 2023 10:10
@xavigibert
Copy link
Contributor Author

Hi @jaeyounkim. I'm trying to send this PR to you for review, but Github is not allowing me to do so. Can you take a look at what is going on?

@laxmareddyp laxmareddyp added the models:research models that come under research directory label Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
models:research models that come under research directory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants