Skip to content

skip gdn core_attn_out check for 8k len due to random numeric error #264

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

Merged
jikunshang merged 1 commit into from
Apr 10, 2026
+1 −1
Merged

skip gdn core_attn_out check for 8k len due to random numeric error #264

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/gdn_attn/test_gdn_attn.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ def test_gdn_attention(num_actual_tokens, batch_size, num_k_heads, head_k_dim,

torch.testing.assert_close(z, ref_z, atol=atol, rtol=rtol)

if ssm_state_is_fp32 and num_actual_tokens == 8192:
if num_actual_tokens == 8192:
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

The PR description template is still unfilled (Purpose/Test Plan/Test Result). Please add at least a short purpose statement and a test command/result, especially since this change relaxes test validation for the 8k-token path.

Copilot uses AI. Check for mistakes.
pytest.skip("FIXME, skip core_attn_out test because of random error")
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

The skip message is very generic ("random error") and doesn’t provide enough context to act on. Consider including the failing symptom (e.g., max diff / which output diverges), the platform (PVC/XPU), and a link to a tracking issue so this doesn’t become a permanent skip.

Copilot uses AI. Check for mistakes.

Comment on lines +408 to 410
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

This change broadens the skip condition from ssm_state_is_fp32 and num_actual_tokens == 8192 to num_actual_tokens == 8192, which significantly reduces coverage for the 8k-token case across all dtypes/state configurations. If the mismatch is only present for specific configurations, keep the skip as narrow as possible; otherwise add a brief note (and ideally a tracking issue/bug ID) explaining why the 8k case is universally skipped.

Copilot uses AI. Check for mistakes.
torch.testing.assert_close(core_attn_out,
Expand Down
Loading