Fix FlashAttention local version validation - #3356
Conversation
Signed-off-by: Vladimir Cherepanov <vcherepanov@nvidia.com>
Greptile SummaryThe PR corrects FlashAttention v2 validation by preserving local version metadata and representing the supported range as a half-open interval.
Confidence Score: 5/5The 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
Reviews (1): Last reviewed commit: "Fix FlashAttention local version validat..." | Re-trigger Greptile |
cyanguwa
left a comment
There was a problem hiding this comment.
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")) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
cyanguwa
left a comment
There was a problem hiding this comment.
Please run CI before merging.
Description
Use half-open interval for FA version checks
Fixes #3334
Type of change
Changes
Please list the changes introduced in this PR:
Checklist: