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

Question about QK-Norm implementation #112

Open
tang-bd opened this issue Nov 1, 2024 · 1 comment
Open

Question about QK-Norm implementation #112

tang-bd opened this issue Nov 1, 2024 · 1 comment

Comments

@tang-bd
Copy link

tang-bd commented Nov 1, 2024

I noticed your implementation of QK-Norm differs from the original paper, where normalization is applied after head splitting of the q, k states. I wonder what's the rationale of your implementation that apply normalization before head splitting? Thanks!

@ChrisLiu6
Copy link
Contributor

ChrisLiu6 commented Nov 5, 2024

There is no special reason for that... it is just something that we had implemented long ago and did not cared too much afterwards. But I agree that applying norm after head splitting looks more reasonable and we may change to that in our following works

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

No branches or pull requests

2 participants