Skip to content

Fix FlashAttention local version validation - #3356

Open
vcherepanov-nv wants to merge 1 commit into
NVIDIA:mainfrom
vcherepanov-nv:fix-fa-local-ver
Open

Fix FlashAttention local version validation#3356
vcherepanov-nv wants to merge 1 commit into
NVIDIA:mainfrom
vcherepanov-nv:fix-fa-local-ver

Conversation

@vcherepanov-nv

Copy link
Copy Markdown
Collaborator

Description

Use half-open interval for FA version checks

Fixes #3334

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:

  • change FA version check logic to use half-open interval
  • tests for version check logic

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

Signed-off-by: Vladimir Cherepanov <vcherepanov@nvidia.com>
@greptile-apps

greptile-apps Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR corrects FlashAttention v2 validation by preserving local version metadata and representing the supported range as a half-open interval.

  • Changes the supported interval to >= 2.1.1, < 2.8.4.
  • Applies the exclusive upper bound consistently during installation detection and backend initialization.
  • Adds tests for final, local, post-release, boundary, and out-of-range versions.

Confidence Score: 5/5

The PR appears safe to merge, with the half-open FlashAttention version boundary applied consistently across detection, validation, messaging, and tests.

The changed support checks correctly admit the intended 2.1.1 through 2.8.3 release range, preserve local metadata without disrupting downstream comparisons, and reject 2.8.4 and later versions.

Important Files Changed

Filename Overview
transformer_engine/pytorch/attention/dot_product_attention/utils.py Introduces a centralized half-open FlashAttention support check and changes the upper-bound sentinel to 2.8.4.
transformer_engine/pytorch/attention/dot_product_attention/backends.py Preserves installed local-version metadata and consistently applies the centralized support check and exclusive constructor assertion.
tests/pytorch/attention/test_attention.py Adds focused coverage for supported versions, local and post-release suffixes, the exclusive boundary, and diagnostic range formatting.

Reviews (1): Last reviewed commit: "Fix FlashAttention local version validat..." | Re-trigger Greptile

@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.

Please run CI to check if everything is in order. Thanks.

# Try to import Flash Attention v2
try:
fa_utils.version = PkgVersion(PkgVersion(get_pkg_version("flash-attn")).public)
fa_utils.version = PkgVersion(get_pkg_version("flash-attn"))

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.

I understand that the issue reported by the user is because 2.8.3+local_version <= 2.8.3 evaluates to False. I think your changes would make that all work, but with .public here, aren't we already stripping away the local versions and return only the 2.8.3 part for comparison correctly?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Probably. But then, the issue is not reported correctly, since 2.8.3+local_version should be handled already? Still, I feel like using half-open interval is more robust, since it handles, say, 2.8.3.post1.

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.

Ok, makes sense.

@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.

Please run CI before merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Runtime check for the FlashAttention version rejects local version identifiers when it shouldn't

2 participants