Skip to content

Conversation

@LoserCheems
Copy link
Collaborator

Adjusts query and key lengths in test configurations to provide more balanced testing scenarios.

Changes small sequence length tests from 4 to 64 tokens to better represent realistic use cases, and modifies the largest configuration to use matching sequence lengths with non-causal attention for improved test diversity.

Adjusts query and key lengths in test configurations to provide more balanced testing scenarios.

Changes small sequence length tests from 4 to 64 tokens to better represent realistic use cases, and modifies the largest configuration to use matching sequence lengths with non-causal attention for improved test diversity.
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 updates benchmark test configurations to improve coverage by adjusting sequence lengths and attention settings.

  • Increased small-sequence tests from 4 to 64 tokens for more realistic scenarios
  • Matched query and key lengths and switched the largest test to non-causal attention
Comments suppressed due to low confidence (2)

benchmarks/benchmark_forward_equivalence.py:360

  • By removing the minimal-length test (query_len=4), we lose an edge-case check for very short sequences; consider retaining at least one small-length test to validate behavior at minimal token counts.
        (1, 1, 1, 64, 64, 32, True),

benchmarks/benchmark_forward_equivalence.py:379

  • Replacing the largest causal test with a non-causal one removes coverage of causal attention at the maximum sequence length; consider including both causal and non-causal variants for the 512×512 configuration.
        (1, 2, 1, 256, 256, 128, True),

@LoserCheems LoserCheems merged commit f006b7a into main Jul 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants