Skip to content

fix: Unreachable backend check after earlier backend skip - #3368

Merged
ptrendx merged 2 commits into
NVIDIA:mainfrom
andrewwhitecdw:codequality/test-fused-attn-unreachable-backend-check-after
Sep 5, 2026
Merged

fix: Unreachable backend check after earlier backend skip#3368
ptrendx merged 2 commits into
NVIDIA:mainfrom
andrewwhitecdw:codequality/test-fused-attn-unreachable-backend-check-after

Conversation

@andrewwhitecdw

@andrewwhitecdw andrewwhitecdw commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

This PR removes an unreachable backend skip from the JAX fused-attention test config checks and adds a regression test.

Root cause

In FusedAttnRunner._check_configs (tests/jax/test_fused_attn.py), the backend is selected via FusedAttnHelper and any backend other than NVTE_Fused_Attn_Backend.NVTE_F16_arbitrary_seqlen is skipped immediately after selection ("Unsupported inputs combination or device compute capability."). Inside the subsequent non-1HSS post-scale-bias block, the elif re-testing self.backend != NVTE_Fused_Attn_Backend.NVTE_F16_arbitrary_seqlen could therefore never be true, making its skip message dead code.

Changes

  • tests/jax/test_fused_attn.py: removed the unreachable elif arm; the sibling padding-mask skip is retained unchanged.
  • tests/jax/test_fused_attn.py: added test_post_scale_non_1hss_bias_shapes_do_not_hit_unreachable_backend_skip, which builds a non-1HSS post-scale-bias config (BiasShape._B1SS, NO_MASK, BSHD layout) whose backend is NVTE_F16_arbitrary_seqlen, calls _check_configs() without hitting a skip, and asserts the selected backend.

Behavior

Behavior on real runs is unchanged: configurations that previously passed still pass, no configuration ever reached the removed arm, and the padding-mask restriction for non-1HSS post-scale bias shapes is preserved.

Testing

Not run locally — the JAX test stack (jax, transformer_engine_jax) is not installed on this machine, so the JAX suite could not be executed here; CI runs it. The added test follows the file's existing pattern of targeting real GPU/cuDNN fused-attention kernels (such tests skip themselves when the hardware/software backend is unavailable).

Branch

Recreated as a single signed-off commit on current main (DCO) in response to the review request.

@github-actions github-actions Bot added the community-contribution PRs from external contributor outside the core maintainers, representing community-driven work. label Aug 13, 2026
@greptile-apps

greptile-apps Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR removes an unreachable fused-attention backend check from the JAX test configuration logic.

  • Keeps the earlier backend gate as the sole backend eligibility check.
  • Preserves the non-1HSS padding-mask restriction.
  • The regression test described by the PR has been removed from the current revision, leaving the changed path without focused collected coverage.

Confidence Score: 5/5

The runtime cleanup appears safe to merge, but restoring focused regression coverage would prevent the unreachable check from being reintroduced unnoticed.

The backend check was redundant behind the existing arbitrary-sequence-length backend gate, and no blocking behavioral defect remains; the sole new concern is the non-blocking loss of targeted regression coverage.

Files Needing Attention: tests/jax/test_fused_attn.py

Important Files Changed

Filename Overview
tests/jax/test_fused_attn.py Removes a redundant backend skip, but also removes the only focused collected regression test for the affected configuration path.

Comments Outside Diff (1)

  1. tests/jax/test_fused_attn.py, line 1414-1447 (link)

    P2 Regression coverage removed

    This deletes the only collected regression test that exercises a non-1HSS post-scale bias with NO_MASK through _check_configs. The remaining non-1HSS cases are in non-collected helper methods, so CI would not detect this unreachable backend check being reintroduced. Please retain a focused collected test for this path.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Reviews (5): Last reviewed commit: "Remove unnecessary test" | Re-trigger Greptile

cyanguwa
cyanguwa previously approved these changes Aug 13, 2026

@cyanguwa cyanguwa left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is a no-brainer. Thanks.

@andrewwhitecdw

Copy link
Copy Markdown
Contributor Author

script closed on accident

1 similar comment
@andrewwhitecdw

Copy link
Copy Markdown
Contributor Author

script closed on accident

@ptrendx

ptrendx commented Sep 1, 2026

Copy link
Copy Markdown
Member

@andrewwhitecdw Please sign your commits.

Root cause: FusedAttnRunner._check_configs skipped with 'Unsupported
inputs combination or device compute capability.' unless the backend was
NVTE_F16_arbitrary_seqlen. The later elif re-testing
self.backend != NVTE_F16_arbitrary_seqlen could therefore never be
reached, so its skip message ('B1SS, BHSS and 11SS bias shapes are only
supported for the F16_arbitrary_seqlen backend') was dead code.

Fix: drop the dead elif arm. The padding-mask skip in the sibling if arm
is retained. A regression test locks the remaining behavior: a
non-1HSS post-scale-bias config (BiasShape._B1SS) that passes
_check_configs selects NVTE_F16_arbitrary_seqlen, proving the removal is
behaviorally invisible and the earlier guard is the sole gate.

Testing: not run locally - the JAX test stack (jax,
transformer_engine_jax) is not installed on this machine. The suite runs
in NVIDIA TransformerEngine CI. Contribution: tests/jax tests target
real GPU/cuDNN fused-attention kernels and skip otherwise; the new test
will follow that path via CI.

Signed-off-by: andrewwhitecdw <andrewwhitecdw@users.noreply.github.com>
@andrewwhitecdw
andrewwhitecdw force-pushed the codequality/test-fused-attn-unreachable-backend-check-after branch from e44fb33 to 98432f3 Compare September 3, 2026 15:15
Signed-off-by: Przemyslaw Tredak <ptrendx@gmail.com>
@ptrendx
ptrendx merged commit a30aee5 into NVIDIA:main Sep 5, 2026
11 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-contribution PRs from external contributor outside the core maintainers, representing community-driven work.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants