Skip to content
This repository was archived by the owner on Aug 30, 2024. It is now read-only.

Add eps as a parameter to ne_norm function #162

Merged
merged 11 commits into from
Mar 13, 2024
Merged

Conversation

aahouzi
Copy link
Member

@aahouzi aahouzi commented Mar 8, 2024

Type of Change

  • eps parameter is hardcoded to 1e-5 in the ne_norm function, and many models in the convert script look for rms_norm_eps while the model doesn't use RMS normalization or have bad naming compared to config.json file.
  • This PR fixes the above issues for all models, and suggests a single norm_eps parameter for both RMS normalization or classic normalization.

Description

  • Same as above

Expected Behavior & Potential Risk

  • N/A

How has this PR been tested?

  • Works fine for all models

Dependency Change?

  • N/A

Copy link
Contributor

@a32543254 a32543254 left a comment

Choose a reason for hiding this comment

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

LGTM

@a32543254
Copy link
Contributor

thanks for you contribution.
After this PR, we could adopt Rms eps and Layernorm eps as one args.
could you kindly also check the Format Scan ?

@aahouzi
Copy link
Member Author

aahouzi commented Mar 12, 2024

@a32543254 all checks are passing now. However, it's little bit weird, since when I apply clang-format on the project directory for my modified files, it didn't pass the clangformat test. When I introduced the commit 7ea5623 , it passed. Actually, I expect the .clang-format file to put everything in place or am I missing smthg here ?

@a32543254
Copy link
Contributor

@a32543254 all checks are passing now. However, it's little bit weird, since when I apply clang-format on the project directory for my modified files, it didn't pass the clangformat test. When I introduced the commit 7ea5623 , it passed. Actually, I expect the .clang-format file to put everything in place or am I missing smthg here ?

you can simple use clang-format.py in repo to auto check the clang format.
Everything is done by now. We will merge this pr soon

@VincyZhang VincyZhang merged commit 0ec1a6e into intel:main Mar 13, 2024
11 checks passed
aahouzi added a commit to aahouzi/neural-speed that referenced this pull request Mar 13, 2024
@aahouzi aahouzi deleted the eps_ne_norm branch March 17, 2024 12:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants