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

replace apex.normalization.FusedLayerNorm with torch.nn.LayerNorm #9386

Merged
merged 1 commit into from
Jan 4, 2021
Merged

replace apex.normalization.FusedLayerNorm with torch.nn.LayerNorm #9386

merged 1 commit into from
Jan 4, 2021

Conversation

stas00
Copy link
Contributor

@stas00 stas00 commented Jan 2, 2021 •
edited
Loading

This PR proposes to drop apex.normalization.FusedLayerNorm in favor of faster torch.nn.LayerNorm.

  1. For performance and background details please see the discussions in replacing apex.normalization.FusedLayerNorm with torch.nn.LayerNorm #9377
  2. It's also needed for [model parallelism] Bart goes parallel #9384 since apex.normalization.FusedLayerNorm corrupts data under model parallel FusedLayerNorm corrupts data when switching devices NVIDIA/apex#1022

Fixes: #9377

@LysandreJik, @sgugger, @patrickvonplaten

@stas00 stas00 mentioned this pull request Jan 2, 2021
Closed
8 tasks
patrickvonplaten
patrickvonplaten approved these changes Jan 3, 2021
View reviewed changes
Copy link
Contributor

@patrickvonplaten patrickvonplaten left a comment

Choose a reason for hiding this comment

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

This looks good to me!

LysandreJik
LysandreJik approved these changes Jan 4, 2021
View reviewed changes
Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

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

LGTM too!

@LysandreJik LysandreJik mentioned this pull request Jan 4, 2021
Merged
15 tasks
sgugger
sgugger approved these changes Jan 4, 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.

Way easier this way, LGTM!

@LysandreJik LysandreJik mentioned this pull request Jan 4, 2021
Merged
15 tasks
@patrickvonplaten
Copy link
Contributor

Merging since it's blocking #9343 .

@patrickvonplaten patrickvonplaten merged commit 47ca0ea into huggingface:master Jan 4, 2021
@stas00 stas00 deleted the native-layer-norm branch January 4, 2021 19:08
guyrosin pushed a commit to guyrosin/transformers that referenced this pull request Jan 15, 2021
@stas00 @guyrosin
replace apex.normalization.FusedLayerNorm with torch.nn.LayerNorm (hu…
d360a48 
…ggingface#9386)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@patrickvonplaten patrickvonplaten patrickvonplaten approved these changes

@LysandreJik LysandreJik LysandreJik approved these changes

@sgugger sgugger sgugger 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.

replacing apex.normalization.FusedLayerNorm with torch.nn.LayerNorm
4 participants
@stas00 @patrickvonplaten @LysandreJik @sgugger