Skip to content

Conversation

@LoserCheems
Copy link
Collaborator

Uncomment test cases for head dimension 128 in both backward and forward equivalence tests, allowing for comprehensive validation across various sequence lengths. Adjust comments regarding shared memory limitations to reflect the current applicability for head dimension 256.

Uncomments previously disabled test configurations for head dimension 128,
allowing comprehensive testing of backward pass equivalence across various
sequence lengths and causal modes.

Moves limitation comment to head dimension 256 section where the restriction
still applies due to splitkv branch constraints and shared memory limitations.
Uncomments previously disabled test cases for head dimension 128 configurations across various sequence lengths.

Moves the comment about sm89 shared memory limitations to head dimension 256 section where it remains applicable.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enables testing for head dimension 128 in both forward and backward equivalence test suites by uncommenting previously disabled test cases. The changes clarify that shared memory limitations now only apply to head dimension 256, not 128.

Key changes:

  • Uncommented 12 test cases for head dimension 128 across various sequence lengths (128-4096)
  • Updated comments to specify that shared memory limitations apply to head dimension 256, not 128

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
benchmarks/forward_equivalence.py Enabled head dimension 128 test cases and updated comments about sm89 limitations
benchmarks/backward_equivalence.py Enabled head dimension 128 test cases and clarified comments about splitkv branch limitations

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +602 to +603
(1, 2, 1, 128, 128, 128, True),
(1, 2, 1, 128, 128, 128, False),
Copy link

Copilot AI Aug 29, 2025

Choose a reason for hiding this comment

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

Line 603 should have False as the last parameter, but the original commented line 603 had True. This creates an inconsistency where there are two test cases with True and none with False for the (1, 2, 1, 128, 128, 128) configuration.

Copilot uses AI. Check for mistakes.
@LoserCheems LoserCheems merged commit e7eb38f into main Aug 29, 2025
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.

7 participants