Skip to content

add production GEMM tests#590

Open
matthiasdiener wants to merge 7 commits into
devfrom
mdiener/prodgemm-test
Open

add production GEMM tests#590
matthiasdiener wants to merge 7 commits into
devfrom
mdiener/prodgemm-test

Conversation

@matthiasdiener
Copy link
Copy Markdown
Contributor

@matthiasdiener matthiasdiener commented May 19, 2026

Description

Add production-like GEMM tests for Qwen3 and Deepseek3.

Type of change

  • Documentation change (change only to the documentation, either a fix or a new content)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Infra/Build change
  • Code refactoring

Changes

Please list the changes introduced in this PR:

  • add a new test suite for production-like GEMM tests for Qwen3 and Deepseek3.

Checklist:

  • I have read and followed the contributing guidelines
  • The functionality is complete
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@matthiasdiener matthiasdiener self-assigned this May 19, 2026
@matthiasdiener matthiasdiener added the ci-level 1 CI test level 1 label May 19, 2026
@matthiasdiener matthiasdiener force-pushed the mdiener/prodgemm-test branch from f3b9d08 to c4c2ea5 Compare May 26, 2026 19:57
@matthiasdiener matthiasdiener requested a review from alextmagro May 27, 2026 19:50
@matthiasdiener matthiasdiener marked this pull request as ready for review May 27, 2026 19:50
Comment thread tests/cpp/operator/test_gemm_prodgemm.cu Outdated
Comment thread tests/cpp/operator/test_gemm_prodgemm.cu Outdated
switch (s.pass) {
case GemmPass::FWD:
case GemmPass::DGRAD:
m = tokens; n = s.dim1; k = s.dim2; break;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: I think break should go on its own line for readability here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, done in 77f1c45

Copy link
Copy Markdown
Collaborator

@ipanfilo ipanfilo left a comment

Choose a reason for hiding this comment

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

Please write the changes description. Also, why production LLM shapes cannot be added to existing gemm test?

@matthiasdiener
Copy link
Copy Markdown
Contributor Author

Please write the changes description.

Added, thanks.

Also, why production LLM shapes cannot be added to existing gemm test?

My thinking was that we can run just these particular tests "standalone", without the other GEMM tests, on (say) a new architecture, for testing.

@ipanfilo
Copy link
Copy Markdown
Collaborator

Please write the changes description.

Added, thanks.

Also, why production LLM shapes cannot be added to existing gemm test?

My thinking was that we can run just these particular tests "standalone", without the other GEMM tests, on (say) a new architecture, for testing.

It the difference is only in usage, can it be added just with different prefix or as different suite: INSTANTIATE_TEST_SUITE_P(OperatorTest, GEMMTestSuite) so they can easy be run separately ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-level 1 CI test level 1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants