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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add custom RMSNorm to ALL_LAYERNORM_LAYERS #26227

Merged
merged 3 commits into from
Sep 20, 2023

Conversation

shijie-wu
Copy link
Contributor

What does this PR do?

It fixed a issue discovered during discussion of PR #26152.

@ArthurZucker: the ALL_LAYERNORM_LAYERS should contain all the custom layer norm classes (from transformers modeling files) and should be updated if that is not the case

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?

Who can review?

@ArthurZucker

Copy link
Collaborator

@ArthurZucker ArthurZucker left a comment

Choose a reason for hiding this comment

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

Thanks! Could you also add IdeficsRMSNorm and MegaRMSNorm? 😉 (make sure make fixup and make fix-copies also pass!)

@shijie-wu
Copy link
Contributor Author

@ArthurZucker done! MegaRMSNorm - a part of MegaSequenceNorm - is already in ALL_LAYERNORM_LAYERS

ALL_LAYERNORM_LAYERS.append(MegaSequenceNorm)

do we want to add all these to ALL_LAYERNORM_LAYERS?

elif norm_type == "scalenorm":
self.norm = MegaScaleNorm(dim=-1, eps=eps, affine=affine)
elif norm_type == "rmsnorm":
self.norm = MegaRMSNorm(embedding_dim, eps=eps, affine=affine)
elif norm_type == "batchnorm":
self.norm = nn.BatchNorm1d(embedding_dim, eps=eps, affine=affine)
elif norm_type == "syncbatchnorm":
self.norm = nn.SyncBatchNorm(embedding_dim, eps=eps, affine=affine)

Copy link
Collaborator

@ArthurZucker ArthurZucker left a comment

Choose a reason for hiding this comment

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

Thanks again! Let's add all of the mega ones as you said!

@shijie-wu shijie-wu changed the title add LlamaRMSNorm to ALL_LAYERNORM_LAYERS add custom RMSNorm to ALL_LAYERNORM_LAYERS Sep 20, 2023
@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.

@shijie-wu
Copy link
Contributor Author

after discussion with @ArthurZucker, it seems best to limit to scope of this PR to adding custom RMSNorm to ALL_LAYERNORM_LAYERS. Adding nn.BatchNorm1d might create unintentional impact.

@ArthurZucker ArthurZucker merged commit e3a4bd2 into huggingface:main Sep 20, 2023
@shijie-wu shijie-wu deleted the llama-decay branch September 20, 2023 21:09
parambharat pushed a commit to parambharat/transformers that referenced this pull request Sep 26, 2023
* add LlamaRMSNorm to ALL_LAYERNORM_LAYERS

* fixup

* add IdeficsRMSNorm to ALL_LAYERNORM_LAYERS and fixup
blbadger pushed a commit to blbadger/transformers that referenced this pull request Nov 8, 2023
* add LlamaRMSNorm to ALL_LAYERNORM_LAYERS

* fixup

* add IdeficsRMSNorm to ALL_LAYERNORM_LAYERS and fixup
EduardoPach pushed a commit to EduardoPach/transformers that referenced this pull request Nov 18, 2023
* add LlamaRMSNorm to ALL_LAYERNORM_LAYERS

* fixup

* add IdeficsRMSNorm to ALL_LAYERNORM_LAYERS and fixup
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