Skip to content

Even more TF test fixes#28146

Merged
Rocketknight1 merged 11 commits intomainfrom
even_more_tf_test_fixes
Dec 21, 2023
Merged

Even more TF test fixes#28146
Rocketknight1 merged 11 commits intomainfrom
even_more_tf_test_fixes

Conversation

@Rocketknight1
Copy link
Copy Markdown
Member

This PR hopefully fixes the last remaining issues from the build() PR and gets the CI back to normal!

for checkpoint in TOKENIZER_CHECKPOINTS
]
self.tokenizers = [BertTokenizer.from_pretrained(checkpoint) for checkpoint in TOKENIZER_CHECKPOINTS]
self.tf_tokenizers = [TFBertTokenizer.from_pretrained(checkpoint) for checkpoint in TOKENIZER_CHECKPOINTS]
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Note to probably-Amy here: I'm not testing the old slow Bert tokenizer here because the fast one seems generally better maintained and is fully supported in TFLite now.

model.save(save_path)
loaded_model = tf.keras.models.load_model(save_path)
loaded_output = loaded_model(test_inputs)
model.export(save_path)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is the recommended TF/Keras way to export models for inference now, and the old approach was always a bit shaky.

@HuggingFaceDocBuilderDev
Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@Rocketknight1
Copy link
Copy Markdown
Member Author

Rocketknight1 commented Dec 20, 2023

This should be ready to go now, and finally fixes the remaining CI issues after the build() PR!

Copy link
Copy Markdown
Contributor

@amyeroberts amyeroberts left a comment

Choose a reason for hiding this comment

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

Thanks for the continued TF tidy-up!

"""
if name_scope is not None:
if not tf_name.startswith(name_scope):
if not tf_name.startswith(name_scope) and "final_logits_bias" not in tf_name:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why the check on "final_logits_bias" here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No good reason, unfortunately! BART and derived models put the final_logits_bias weights outside the main model name scope, and I'm not entirely sure why (but changing it now might invalidate old user checkpoints)

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

Successfully merging this pull request may close these issues.

3 participants