Skip to content

PR_010 - CodeRabbit #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
MitchLewis930 wants to merge 1 commit into from
Closed

PR_010 - CodeRabbit #12

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tests/models/language/pooling/test_token_classification.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ def test_modernbert_models(
for hf_output, vllm_output in zip(hf_outputs, vllm_outputs):
hf_output = hf_output.detach().clone().cpu().float()
vllm_output = vllm_output.detach().clone().cpu().float()
torch.testing.assert_close(hf_output, vllm_output, atol=1.2e-2, rtol=1e-3)
assert torch.allclose(hf_output, vllm_output, atol=1e-2)
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Jan 14, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Reverting from torch.testing.assert_close to torch.allclose degrades test quality and may cause flakiness.

This change introduces two issues:

  1. Loss of diagnostics: torch.testing.assert_close provides detailed error messages showing which elements differ and by how much. Using assert torch.allclose(...) only raises a bare AssertionError with no useful debugging information when the test fails.

  2. Stricter tolerance: The original used atol=1.2e-2, rtol=1e-3. The new code uses atol=1e-2 with default rtol=1e-5 — this is stricter on both absolute (20% reduction) and relative (100× stricter) tolerances. Given this test exists specifically to handle Triton kernel numerical differences (per the upstream PR #31776 this reverts), tighter tolerances risk intermittent test failures.

Suggested fix 
-        assert torch.allclose(hf_output, vllm_output, atol=1e-2)
+        torch.testing.assert_close(hf_output, vllm_output, atol=1.2e-2, rtol=1e-3)
📝 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.

Suggested change
assert torch.allclose(hf_output, vllm_output, atol=1e-2)
torch.testing.assert_close(hf_output, vllm_output, atol=1.2e-2, rtol=1e-3)
🤖 Prompt for AI Agents 
In `@tests/models/language/pooling/test_token_classification.py` at line 81,
Replace the muted bare assertion using torch.allclose with
torch.testing.assert_close to restore detailed diagnostics and the original
tolerances; specifically, in the test assertion that compares hf_output and
vllm_output (the line currently using torch.allclose), call
torch.testing.assert_close(hf_output, vllm_output, atol=1.2e-2, rtol=1e-3) so
the test uses the intended looser absolute and relative tolerances and emits
helpful element-level difference messages on failure.



@pytest.mark.parametrize("model", ["bd2lcco/Qwen3-0.6B-finetuned"])
Expand Down