fix test error regarding logits_types#2918
Conversation
aleozlx
requested review from
IwakuraRein,
jiahanc,
nv-yunzheq,
samuellees and
yzh119
as code owners
|
/bot run |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates several test cases in tests/moe/test_trtllm_gen_fused_moe.py by explicitly specifying the logits_dtype parameter as either torch.bfloat16 or torch.float32 across various MoE test functions. I have no feedback to provide as there were no review comments to evaluate.
📝 WalkthroughWalkthroughAdded explicit Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
aleozlx
requested review from
bkryu,
cyx-6,
jimmyzho,
kahyunnam,
saltyminty,
sricketts,
yongwww and
yyihuang
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/model_optimizations/test_tinygemm2.py`:
- Around line 6-8: Replace the unconditional module-wide skip by using
flashinfer.utils.get_compute_capability() to only skip on CI H100 runners:
remove the current pytestmark = pytest.mark.skip(...) and instead set pytestmark
= pytest.mark.skipif(<condition>, reason=...) where <condition> checks
get_compute_capability() == "h100" (and optionally that CI is set, e.g.,
os.environ.get("CI") is truthy) so tests still run locally; import
get_compute_capability from flashinfer.utils and os if you use the CI env check
and reference the pytestmark symbol in this file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: be85d081-a97f-4ce0-9f06-8ef5f8df4cf7
📒 Files selected for processing (1)
tests/model_optimizations/test_tinygemm2.py
| pytestmark = pytest.mark.skip( | ||
| reason="tinygemm2 hangs on CI H100 runners — investigation in progress" | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether tinygemm2 is globally skipped vs conditionally skipped.
# Expected: no unconditional module-level "pytest.mark.skip(" in this file.
rg -n --type=py '^\s*pytestmark\s*=\s*pytest\.mark\.skip\(' tests/model_optimizations/test_tinygemm2.py
rg -n --type=py '^\s*pytestmark\s*=\s*pytest\.mark\.skipif\(' tests/model_optimizations/test_tinygemm2.py
rg -n --type=py 'get_compute_capability|is_sm90a_supported|is_sm100a_supported' tests/model_optimizations/test_tinygemm2.pyRepository: flashinfer-ai/flashinfer
Length of output: 211
Replace global module skip with conditional CI/H100 skip.
Line 6 disables all tinygemm2 tests everywhere, removing useful regression coverage. Per coding guidelines, use flashinfer.utils.get_compute_capability() to conditionally skip only on the problematic CI H100 environment.
Proposed change
+import os
import torch
import pytest
import torch.nn.functional as F
from flashinfer.utils import get_compute_capability
-pytestmark = pytest.mark.skip(
- reason="tinygemm2 hangs on CI H100 runners — investigation in progress"
-)
+def _is_ci_h100():
+ cc = get_compute_capability(torch.device("cuda"))
+ return os.getenv("CI") == "true" and cc[0] == 9
+
+pytestmark = pytest.mark.skipif(
+ _is_ci_h100(),
+ reason="tinygemm2 hangs on CI H100 runners — investigation in progress",
+)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pytestmark = pytest.mark.skip( | |
| reason="tinygemm2 hangs on CI H100 runners — investigation in progress" | |
| ) | |
| import os | |
| import torch | |
| import pytest | |
| import torch.nn.functional as F | |
| from flashinfer.utils import get_compute_capability | |
| def _is_ci_h100(): | |
| cc = get_compute_capability(torch.device("cuda")) | |
| return os.getenv("CI") == "true" and cc[0] == 9 | |
| pytestmark = pytest.mark.skipif( | |
| _is_ci_h100(), | |
| reason="tinygemm2 hangs on CI H100 runners — investigation in progress", | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/model_optimizations/test_tinygemm2.py` around lines 6 - 8, Replace the
unconditional module-wide skip by using
flashinfer.utils.get_compute_capability() to only skip on CI H100 runners:
remove the current pytestmark = pytest.mark.skip(...) and instead set pytestmark
= pytest.mark.skipif(<condition>, reason=...) where <condition> checks
get_compute_capability() == "h100" (and optionally that CI is set, e.g.,
os.environ.get("CI") is truthy) so tests still run locally; import
get_compute_capability from flashinfer.utils and os if you use the CI env check
and reference the pytestmark symbol in this file.
|
[FAILED] Pipeline #47302910: 10/20 passed |
4 participants
📌 Description
🔍 Related Issues
#2534
(to clarify, not that PR's fault entirely but a CI race condition allowing it to merge)
🚀 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
pre-commitby runningpip install pre-commit(or used your preferred method).pre-commit install.pre-commit run --all-filesand fixed any reported issues.🧪 Tests
unittest, etc.).Reviewer Notes
Summary by CodeRabbit