Skip to content

refactor: Allow models to override apply_qk_norm. #4078

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
Merged
merged 2 commits into from
May 12, 2025
Merged

refactor: Allow models to override apply_qk_norm. #4078

merged 2 commits into from
May 12, 2025

Conversation

yuxianq
Copy link
Collaborator

@yuxianq yuxianq commented May 6, 2025

No description provided.

@yuxianq yuxianq requested review from byshiue, chang-l, hlu1 and mikeiovine May 6, 2025 05:47
@yuxianq
Copy link
Collaborator Author

yuxianq commented May 6, 2025

/bot run --disable-fail-fast

@tensorrt-cicd
Copy link
Collaborator

PR_Github #4152 [ run ] triggered by Bot

hlu1
hlu1 reviewed May 6, 2025
View reviewed changes
@yuxianq
Copy link
Collaborator Author

yuxianq commented May 6, 2025

/bot run --disable-fail-fast

@tensorrt-cicd
Copy link
Collaborator

PR_Github #4172 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #4152 [ run ] completed with state ABORTED

byshiue
byshiue approved these changes May 6, 2025
View reviewed changes
Copy link
Collaborator

@byshiue byshiue left a comment

Choose a reason for hiding this comment

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

Looks good on qwen3 side.

@tensorrt-cicd
Copy link
Collaborator

PR_Github #4172 [ run ] completed with state FAILURE
/LLM/main/L0_MergeRequest_PR pipeline #2980 completed with status: 'FAILURE'

hlu1
hlu1 reviewed May 6, 2025
View reviewed changes
@yuxianq
Copy link
Collaborator Author

yuxianq commented May 7, 2025

/bot run --disable-fail-fast

@tensorrt-cicd
Copy link
Collaborator

PR_Github #4301 [ run ] triggered by Bot

@yuxianq
Copy link
Collaborator Author

yuxianq commented May 7, 2025

/bot run --disable-fail-fast

@tensorrt-cicd
Copy link
Collaborator

PR_Github #4303 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #4301 [ run ] completed with state ABORTED

chang-l
chang-l reviewed May 7, 2025
View reviewed changes
chang-l
chang-l reviewed May 7, 2025
View reviewed changes
chang-l
chang-l approved these changes May 7, 2025
View reviewed changes
Copy link
Collaborator

@chang-l chang-l left a comment

Choose a reason for hiding this comment

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

LGTM

hlu1
hlu1 reviewed May 7, 2025
View reviewed changes
@yuxianq
Copy link
Collaborator Author

yuxianq commented May 7, 2025

/bot run --disable-fail-fast

@tensorrt-cicd
Copy link
Collaborator

PR_Github #4330 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #4303 [ run ] completed with state ABORTED

@tensorrt-cicd
Copy link
Collaborator

PR_Github #4330 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #3104 completed with status: 'FAILURE'

@hlu1
Copy link
Collaborator

hlu1 commented May 7, 2025

We should add more attention unit tests to test these features. Relying on e2e accuracy tests is too heavy and it takes time to debug the accuracy issues.

@yuxianq
Copy link
Collaborator Author

yuxianq commented May 8, 2025

/bot run --disable-fail-fast

1 similar comment
@yuxianq
Copy link
Collaborator Author

yuxianq commented May 8, 2025

/bot run --disable-fail-fast

@tensorrt-cicd
Copy link
Collaborator

PR_Github #4462 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #4462 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #3201 completed with status: 'FAILURE'

@yuxianq
Copy link
Collaborator Author

yuxianq commented May 8, 2025

/bot run --disable-fail-fast

@yuxianq yuxianq enabled auto-merge (squash) May 8, 2025 06:37
@tensorrt-cicd
Copy link
Collaborator

PR_Github #4508 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #4508 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #3241 completed with status: 'FAILURE'

@yuxianq
Copy link
Collaborator Author

yuxianq commented May 8, 2025

/bot run --disable-fail-fast

@tensorrt-cicd
Copy link
Collaborator

PR_Github #4595 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #4595 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #3309 completed with status: 'FAILURE'

@yuxianq yuxianq force-pushed the refactor-qk-norm branch from 493a21c to 1c4c512 Compare May 12, 2025 02:38
@yuxianq
Copy link
Collaborator Author

yuxianq commented May 12, 2025

/bot run --disable-fail-fast

@yuxianq yuxianq mentioned this pull request May 12, 2025
Closed
@yuxianq yuxianq requested a review from dongjiyingdjy May 12, 2025 02:41
@tensorrt-cicd
Copy link
Collaborator

PR_Github #4811 [ run ] triggered by Bot

@dongjiyingdjy
Copy link
Collaborator

Looks good on qwen3.

@tensorrt-cicd
Copy link
Collaborator

PR_Github #4811 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #3482 completed with status: 'SUCCESS'

@yuxianq yuxianq merged commit b35f9a6 into NVIDIA:main May 12, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chang-l chang-l chang-l approved these changes

@byshiue byshiue byshiue approved these changes

@hlu1 hlu1 hlu1 approved these changes

@dongjiyingdjy dongjiyingdjy dongjiyingdjy approved these changes

@mikeiovine mikeiovine Awaiting requested review from mikeiovine

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@yuxianq @tensorrt-cicd @hlu1 @dongjiyingdjy @chang-l @byshiue