Skip to content

[PyTorch] Fix L3 FA tests#2709

Merged
cyanguwa merged 3 commits intoNVIDIA:mainfrom
cyanguwa:fix_L3_FA
Feb 28, 2026
Merged

[PyTorch] Fix L3 FA tests#2709
cyanguwa merged 3 commits intoNVIDIA:mainfrom
cyanguwa:fix_L3_FA

Conversation

@cyanguwa
Copy link
Collaborator

@cyanguwa cyanguwa commented Feb 26, 2026

Description

This PR fixes the L3 tests for FP8 current scaling in L3_pytorch_FA_versions_test. The fix is only related to the selection logic in the test and not the backend support itself.

Type of change

  • Documentation change (change only to the documentation, either a fix or a new content)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Infra/Build change
  • Code refactoring

Changes

Please list the changes introduced in this PR:

  • L3 CI test fix.

Checklist:

  • I have read and followed the contributing guidelines
  • The functionality is complete
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

cyanguwa and others added 2 commits February 26, 2026 10:58
Signed-off-by: Charlene Yang <8636796+cyanguwa@users.noreply.github.com>
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 26, 2026

Greptile Summary

Fixed backend availability checking logic in FP8 attention tests (test_mha_fp8_vs_f16 and test_dpa_fp8_vs_f16). Previously, F16 reference backend availability was only checked when fp8_dpa_bwd=False, which could cause test failures when fp8_dpa_bwd=True if the F16 backend wasn't available.

Key changes:

  • Both tests now unconditionally check availability of FP8 and F16 backends using two separate calls to get_available_attention_backends
  • Improved variable naming: fused_attn_supported split into fused_attn_supported_fp8 and fused_attn_supported_f16 for clarity
  • Test execution and comparisons properly gated by checking which backends are actually available
  • More descriptive skip message: "No reference backend available" when F16 backend is missing

Confidence Score: 5/5

  • This PR is safe to merge with no risk
  • Test-only changes that fix incorrect skip logic without modifying any production code or backend implementation. The changes ensure proper backend availability checking and correct test gating
  • No files require special attention

Important Files Changed

Filename Overview
tests/pytorch/attention/test_attention.py Fixed backend availability checks to always verify both FP8 and F16 backends regardless of fp8_dpa_bwd flag, ensuring proper test execution and skipping

Last reviewed commit: 229b1ef

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

@cyanguwa
Copy link
Collaborator Author

/te-ci pytorch L3

)
_, fused_attn_supported, _ = available_backends
if not fused_attn_supported:
_, fused_attn_supported_f16, _ = available_backends
Copy link
Collaborator

Choose a reason for hiding this comment

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

So if the value for fused_attn_supported_fp8=1/True and fp8_dpa_bwd=0/False then fused_attn_supported_f16 be 0/False or 1/True right ?
If fused_attn_supported_f16 is 0/False, there's no fp16 to compare the fp8 and hence we skip, but
,if fused_attn_supported_f16 is 1/True, then compare fp8 and fp16 fwd only.

Now, if fp8_dpa_bwd=1/True, then fused_attn_supported_f16 will be False/0 always
In that case what does fp8 get compared to ? Maybe I missed it, but I see no logic for this comparison - could you point me to it ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Makes sense. I removed the if not fp8_bwd logic. Could you please take a look at 229b1ef?

Signed-off-by: Charlene Yang <8636796+cyanguwa@users.noreply.github.com>
@cyanguwa
Copy link
Collaborator Author

/te-ci pytorch L3

Copy link
Collaborator

@KshitijLakhani KshitijLakhani left a comment

Choose a reason for hiding this comment

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

LGTM !
Thanks !

@cyanguwa
Copy link
Collaborator Author

Pipeline 44991844.

@cyanguwa cyanguwa merged commit 3ecb5bf into NVIDIA:main Feb 28, 2026
21 of 26 checks passed
KshitijLakhani pushed a commit that referenced this pull request Feb 28, 2026
* fix L3 FA fp8 tests

Signed-off-by: Charlene Yang <8636796+cyanguwa@users.noreply.github.com>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix skip logic based on reference backend

Signed-off-by: Charlene Yang <8636796+cyanguwa@users.noreply.github.com>

---------

Signed-off-by: Charlene Yang <8636796+cyanguwa@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants