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

update layernorm #1570

Open
wants to merge 22 commits into
base: develop
Choose a base branch
from
Open

update layernorm #1570

wants to merge 22 commits into from

Conversation

ltqin
Copy link
Collaborator

@ltqin ltqin commented Oct 12, 2024

No description provided.

aska-0096
aska-0096 previously approved these changes Oct 12, 2024
Copy link
Contributor

@aska-0096 aska-0096 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@carlushuang carlushuang left a comment

Choose a reason for hiding this comment

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

  1. API naming issue, we can't have layernorm2d_fwd_fp16, layernorm2d_fwd_fp16. Need have a single API layernorm2d_fwd which is originally rocking version
  2. need use trait to dispatch API. This PR completely remove this
  3. p_mean, p_invStd removed??
    -> this PR need more change to merge to public

2. Move construction of tensor_view and tile_window to operator()
spolifroni-amd
spolifroni-amd previously approved these changes Oct 15, 2024
Copy link
Contributor

@spolifroni-amd spolifroni-amd left a comment

Choose a reason for hiding this comment

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

Nothing to do on docs side.

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.

5 participants