fix: Skipping attention sink Blackwell test outside of Blackwell by bkryu · Pull Request #1978 · flashinfer-ai/flashinfer

bkryu · 2025-10-24T21:11:46Z

📌 Description

test_attention_sink_blackwell.py checks flashinfer.prefill.trtllm_batch_context_with_kv_cache and flashinfer.decode.trtllm_batch_decode_with_kv_cache which are only supported on Blackwell SM100 and SM103.

Existing check only skips testing of SM 11x or 12x, which causes failures on Hopper SM90.

Test outputs:

H200:
- Before Fix: 144 failed, 1 warning in 9.20s
- After Fix: 144 skipped, 1 warning in 0.42s
B200:
- After Fix: 144 passed in 34.64s

🔍 Related Issues

🚀 Pull Request Checklist

Thank you for contributing to FlashInfer! Before we review your pull request, please make sure the following items are complete.

✅ Pre-commit Checks

I have installed pre-commit by running pip install pre-commit (or used your preferred method).
I have installed the hooks with pre-commit install.
I have run the hooks manually with pre-commit run --all-files and fixed any reported issues.

If you are unsure about how to set up pre-commit, see the pre-commit documentation.

🧪 Tests

Tests have been added or updated as needed.
All tests are passing (unittest, etc.).

Reviewer Notes

Summary by CodeRabbit

Tests
- Updated GPU compatibility checks for attention sink tests to target specific GPU architectures (SM100/SM103). Tests now run exclusively on supported GPU models with updated filtering criteria.

coderabbitai · 2025-10-24T21:11:54Z

Walkthrough

Modified GPU compatibility check in attention sink tests to restrict execution only to SM100/SM103 GPUs by changing the skip condition from excluding SM110/SM120/SM121 to rejecting all compute capabilities except compute_capability[0] == 10.

Changes

Cohort / File(s)	Summary
GPU Skip Condition Update `tests/attention/test_attention_sink_blackwell.py`	Modified test skip conditions to restrict execution to SM100/SM103 GPUs only (compute_capability[0] == 10); updated skip messages for affected test functions

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Single file with straightforward conditional logic modification
Skip condition change is localized and easy to verify

Suggested reviewers

yzh119
nvmbreughe
PerkzZheng

Poem

🐰 Hops through tests with GPU cheer,
SM100, SM103 draw near!
Skip the rest with careful care,
Blackwell tasks run bright and fair! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The PR title "fix: Skipping attention sink Blackwell test outside of Blackwell" is directly related to the changeset, which updates the skip condition in `test_attention_sink_blackwell.py` to properly exclude non-Blackwell GPUs. The title is concise, clear, and specific—it clearly identifies that the fix addresses the skip logic for Blackwell tests when running on non-Blackwell hardware. The title accurately summarizes the main change without being misleading or overly vague.
Description Check	✅ Passed	The PR description follows the repository template structure and includes all critical sections. The Description section is well-populated with an explanation of what the test verifies, why the fix was needed (the original check failed on Hopper SM90), and empirical evidence showing test results before and after the fix. The Pre-commit Checks and Tests sections in the Pull Request Checklist are all marked complete. Optional sections like Related Issues and Reviewer Notes are present but empty, which is acceptable. The description provides sufficient context and demonstrates that the necessary checks were performed.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

tests/attention/test_attention_sink_blackwell.py (1)
144-145: LGTM! Consider harmonizing skip messages.

The skip condition fix is correct and consistent with the first test. However, the skip message differs slightly from line 44 ("trtllm-gen only supports" vs. "These tests are only guaranteed to work on"). Consider using consistent wording across both tests.

Optional: Harmonize the message with line 44 for consistency:
-        pytest.skip("These tests are only guaranteed to work on SM100 and SM103 GPUs.")
+        pytest.skip("trtllm-gen only supports SM100 and SM103 GPUs.")

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a565294 and 754c430.

📒 Files selected for processing (1)

tests/attention/test_attention_sink_blackwell.py (2 hunks)

🔇 Additional comments (1)

tests/attention/test_attention_sink_blackwell.py (1)

43-44: LGTM! Skip logic correctly fixed.

The change from a blacklist approach (in [11, 12]) to a whitelist approach (!= 10) correctly ensures tests only run on SM100 and SM103 GPUs (compute capability 10.x), preventing execution on incompatible architectures like Hopper SM90.

…shinfer-ai#1978)  `test_attention_sink_blackwell.py` checks `flashinfer.prefill.trtllm_batch_context_with_kv_cache` and `flashinfer.decode.trtllm_batch_decode_with_kv_cache` which are only supported on Blackwell SM100 and SM103. Existing check only skips testing of SM 11x or 12x, which causes failures on Hopper SM90. Test outputs: * H200: * Before Fix: `144 failed, 1 warning in 9.20s` * After Fix: `144 skipped, 1 warning in 0.42s` * B200: * After Fix: `144 passed in 34.64s `   Thank you for contributing to FlashInfer! Before we review your pull request, please make sure the following items are complete. - [x] I have installed `pre-commit` by running `pip install pre-commit` (or used your preferred method). - [x] I have installed the hooks with `pre-commit install`. - [x] I have run the hooks manually with `pre-commit run --all-files` and fixed any reported issues. > If you are unsure about how to set up `pre-commit`, see [the pre-commit documentation](https://pre-commit.com/). - [x] Tests have been added or updated as needed. - [x] All tests are passing (`unittest`, etc.).   * **Tests** * Updated GPU compatibility checks for attention sink tests to target specific GPU architectures (SM100/SM103). Tests now run exclusively on supported GPU models with updated filtering criteria.

Skipping attention sink blackwell test outside of blackwell

754c430

bkryu marked this pull request as ready for review October 24, 2025 21:13

coderabbitai Bot reviewed Oct 24, 2025

View reviewed changes

yzh119 approved these changes Oct 24, 2025

View reviewed changes

yzh119 enabled auto-merge (squash) October 24, 2025 22:52

yzh119 merged commit 77091d4 into flashinfer-ai:main Oct 24, 2025
4 checks passed

bkryu deleted the test_trtllm_attn_blackwell_skip branch October 30, 2025 17:18

coderabbitai Bot mentioned this pull request Nov 7, 2025

test: Skip unsupported SM Archs for newly added trtllm MoE test #2060

Merged

5 tasks

coderabbitai Bot mentioned this pull request Jan 30, 2026

Skip trtllm_alltoall tests on Thor #2448

Merged

5 tasks

coderabbitai Bot mentioned this pull request May 8, 2026

[chore] Add guard to blackwell GDN prefill #3267

Open

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Skipping attention sink Blackwell test outside of Blackwell#1978

fix: Skipping attention sink Blackwell test outside of Blackwell#1978
yzh119 merged 1 commit intoflashinfer-ai:mainfrom
bkryu:test_trtllm_attn_blackwell_skip

bkryu commented Oct 24, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 24, 2025 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

bkryu commented Oct 24, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📌 Description

🔍 Related Issues

🚀 Pull Request Checklist

✅ Pre-commit Checks

🧪 Tests

Reviewer Notes

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bkryu commented Oct 24, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 24, 2025 •

edited

Loading