Skip to content

[TRITON] Reduce GEMM unit tests#2584

Merged
vgokhale merged 10 commits intomainfrom
vinayak/reduce_aiter_uts
Apr 3, 2026
Merged

[TRITON] Reduce GEMM unit tests#2584
vgokhale merged 10 commits intomainfrom
vinayak/reduce_aiter_uts

Conversation

@vgokhale
Copy link
Copy Markdown
Contributor

@vgokhale vgokhale commented Apr 1, 2026

Reduce UTs by removing unnecessary tests. This should be a reduction of ~88%. Mainly done by

  1. Reduce number of shapes and keep the relevant ones
  2. Have another set of smaller shapes to use for different layouts, output tensor arg and float16.

@vgokhale vgokhale requested review from a team, azaidy, brunomazzottiamd and k50112113 April 1, 2026 17:59
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 1, 2026

🏷️ CI Guide

Runs automatically on every PR:

  • ✅ Pre-checks (submodule verification, code formatting)
  • ✅ Aiter op tests (gfx942 + gfx950)
  • ✅ Triton tests (only when aiter/ops/triton/** or related paths are changed)

Extended tests (opt-in via labels):

Label Tests
ci:triton-355 Run Triton tests on MI355 in addition to MI325
ci:sglang SGLang integration tests
ci:atom ATOM benchmark (DeepSeek-R1 + GPT-OSS)
ci:vllm vLLM benchmark
ci:all All of the above

Add labels via the sidebar or gh pr edit 2584 --add-label <label>

@vgokhale vgokhale force-pushed the vinayak/reduce_aiter_uts branch from c484bbe to c9be435 Compare April 1, 2026 18:52
brunomazzottiamd
brunomazzottiamd previously approved these changes Apr 1, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@brunomazzottiamd brunomazzottiamd left a comment

Choose a reason for hiding this comment

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

Notice: Black formatting is failing.

Question: Why changing GEMM configs in this PR? My intuition says it's performance related and it should be done. However, the PR description doesn't mention anything about GEMM configs. Is there any relationship between the configs and the test case reduction?

I've also left some nitpick suggestions (address them at your own judgment) and some questions about the intersection with #2491 (I'd be happy to hear your opinion).

Comment thread aiter/ops/triton/gluon/gemm_afp4wfp4.py Outdated
Comment thread aiter/ops/triton/gluon/gemm_a8w8.py
Comment thread op_tests/triton_tests/gemm/basic/test_gemm_a16w16.py Outdated
Comment thread op_tests/triton_tests/gemm/basic/test_gemm_a16w16.py
Comment thread op_tests/triton_tests/gemm/basic/test_gemm_a16w16_gated.py
Comment thread op_tests/triton_tests/gemm/basic/test_gemm_a16wfp4.py Outdated
Comment thread op_tests/triton_tests/gemm/basic/test_gemm_afp4wfp4.py Outdated
Comment thread op_tests/triton_tests/gemm/basic/test_gemm_a8wfp4.py Outdated
@vgokhale
Copy link
Copy Markdown
Contributor Author

vgokhale commented Apr 2, 2026

Question: Why changing GEMM configs in this PR? My intuition says it's performance related and it should be done. However, the PR description doesn't mention anything about GEMM configs. Is there any relationship between the configs and the test case reduction?

@brunomazzottiamd it is because the UTs were failing due to async copies on. @azaidy PR should properly fix them. I just changed them now to get past the failures.

@brunomazzottiamd
Copy link
Copy Markdown
Contributor

brunomazzottiamd commented Apr 2, 2026

Question: Why changing GEMM configs in this PR? My intuition says it's performance related and it should be done. However, the PR description doesn't mention anything about GEMM configs. Is there any relationship between the configs and the test case reduction?

@brunomazzottiamd it is because the UTs were failing due to async copies on. @azaidy PR should properly fix them. I just changed them now to get past the failures.

Now I understand why you touched the configs. Async copy issues shouldn't be a blocker for merging this PR IMHO:

brunomazzottiamd
brunomazzottiamd previously approved these changes Apr 2, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@brunomazzottiamd brunomazzottiamd left a comment

Choose a reason for hiding this comment

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

Let's merge it!

k50112113
k50112113 reviewed Apr 2, 2026
View reviewed changes
Comment thread op_tests/triton_tests/gemm/basic/test_gemm_a16wfp4.py
Comment thread op_tests/triton_tests/gemm/basic/test_gemm_afp4wfp4.py
@brunomazzottiamd brunomazzottiamd mentioned this pull request Apr 2, 2026
Merged
1 task
azaidy
azaidy approved these changes Apr 2, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@azaidy azaidy left a comment

Choose a reason for hiding this comment

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

LGTM!

azaidy
azaidy previously approved these changes Apr 2, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@azaidy azaidy left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment thread op_tests/triton_tests/gemm/basic/test_gemm_a8w8.py Outdated
Comment thread op_tests/triton_tests/gemm/basic/test_gemm_a8w8.py Outdated
@vgokhale vgokhale dismissed stale reviews from azaidy and brunomazzottiamd via 3d3748a April 2, 2026 23:14
@vgokhale vgokhale requested a review from azaidy April 2, 2026 23:17
azaidy
azaidy approved these changes Apr 3, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@azaidy azaidy left a comment

Choose a reason for hiding this comment

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

LGTM!

@vgokhale vgokhale merged commit 6c2813a into main Apr 3, 2026
36 of 38 checks passed
@vgokhale vgokhale deleted the vinayak/reduce_aiter_uts branch April 3, 2026 01:45
yzhou103 pushed a commit that referenced this pull request Apr 8, 2026
@vgokhale @yzhou103
[TRITON] Reduce GEMM unit tests (#2584)
d5e44d9 
Reduce UTs by removing unnecessary tests. This should be a reduction of ~88%. Mainly done by

Reduce number of shapes and keep the relevant ones
Have another set of smaller shapes to use for different layouts, output tensor arg and float16.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@k50112113 k50112113 k50112113 left review comments

@azaidy azaidy azaidy approved these changes

@brunomazzottiamd brunomazzottiamd brunomazzottiamd left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vgokhale @brunomazzottiamd @azaidy @k50112113