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.

Sign up for GitHub

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

Jump to bottom

Rework the AMP for TF XLNet #10274

Merged
merged 1 commit into from
Feb 24, 2021
Merged

Rework the AMP for TF XLNet #10274

merged 1 commit into from
Feb 24, 2021

Conversation

jplu
Copy link
Contributor

@jplu jplu commented Feb 19, 2021

What does this PR do?

This PR reworks the AMP of XLNet to remove some useless casts for better and less confusing AMP compliancy.

sgugger
sgugger reviewed Feb 19, 2021
View reviewed changes
Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

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

Wondering since there are bfloat16 tests here. Does the mixed precision handle bfloat16 on TPUs? If not aren't we removing this functionality here?

@jplu
Copy link
Contributor Author

jplu commented Feb 19, 2021

Yes, bfloat16 is only for TPU. Hence, we cannot really test it elsewhere than inside a TPU context. I have added the bfloat16 condition only if XLNet is run on TPU because we were handling a specific case when the model is run under AMP.

@LysandreJik LysandreJik merged commit cdcdd5f into huggingface:master Feb 24, 2021
@jplu jplu deleted the tf-xlnet-amp branch February 25, 2021 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sgugger sgugger sgugger left review comments

@LysandreJik LysandreJik LysandreJik approved these changes

@patrickvonplaten patrickvonplaten patrickvonplaten approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jplu @patrickvonplaten @LysandreJik @sgugger