Skip to content

[Bugfix][PyTorch][FlashAttention][CP] Honor requested FA padding in CP tests - #3269

Merged
sudhakarsingh27 merged 2 commits into
NVIDIA:mainfrom
sudhakarsingh27:fix/fa3-cp-pad-between-seqs
Jul 29, 2026
Merged

[Bugfix][PyTorch][FlashAttention][CP] Honor requested FA padding in CP tests#3269
sudhakarsingh27 merged 2 commits into
NVIDIA:mainfrom
sudhakarsingh27:fix/fa3-cp-pad-between-seqs

Conversation

@sudhakarsingh27

Copy link
Copy Markdown
Member

The CP test runner creates inter-sequence padding when the FlashAttention padding case is requested, but it unconditionally told DPA that FlashAttention THD inputs had no padding. That mismatch left CP backward padding uninitialized.

Derive the explicit padding state from the same condition used to generate the inputs and reuse it for both reference and CP calls, preserving the sync-free CUDA-graph path for non-padding cases.

Description

Please include a brief summary of the changes, relevant motivation and context.

Fixes # (issue)

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 A
  • Change B

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

The CP test runner creates inter-sequence padding when the FlashAttention padding case is requested, but it unconditionally told DPA that FlashAttention THD inputs had no padding. That mismatch left CP backward padding uninitialized.\n\nDerive the explicit padding state from the same condition used to generate the inputs and reuse it for both reference and CP calls, preserving the sync-free CUDA-graph path for non-padding cases.

Signed-off-by: Sudhakar Singh <sudhakars@nvidia.com>
@sudhakarsingh27

Copy link
Copy Markdown
Member Author

@HaochenYuan, #2898 recently changed some assumptions with FlashAttention which broke CI, this is a fix, could you confirm/review that these changes don't affect your use-case? Thanks!

@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Updates the context-parallel attention test runner to derive the explicit THD inter-sequence padding state from the same condition used to generate inputs, then passes that state consistently to both reference and CP attention calls.

Confidence Score: 5/5

The PR appears safe to merge with no actionable defects identified.

The new explicit padding state exactly matches the existing THD input-generation condition, preserves None-based handling for non-THD formats, and is reused consistently by both reference and context-parallel calls.

Important Files Changed

Filename Overview
tests/pytorch/attention/run_attention_with_cp.py The shared padding flag matches input generation for FlashAttention and FusedAttention and is consistently applied to both execution paths.

Reviews (1): Last reviewed commit: "[pre-commit.ci] auto fixes from pre-comm..." | Re-trigger Greptile

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes incorrect pad_between_seqs signaling in PyTorch Context Parallel (CP) attention tests for THD inputs when FlashAttention inter-sequence padding is requested, ensuring DPA/CP see the same explicit padding state used to generate inputs (and avoiding the conservative auto-detect behavior that can alter backend eligibility).

Changes:

  • Derive pad_between_seqs for qkv_format="thd" from the same condition used in generate_input_shapes.
  • Reuse the derived pad_between_seqs for both the non-CP reference call and the CP call to avoid mismatched padding handling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sudhakarsingh27

Copy link
Copy Markdown
Member Author

/te-ci pytorch L3

@sudhakarsingh27

Copy link
Copy Markdown
Member Author

/te-ci pytorch L3

@sudhakarsingh27 sudhakarsingh27 self-assigned this Jul 28, 2026
@sudhakarsingh27
sudhakarsingh27 merged commit 64da9f5 into NVIDIA:main Jul 29, 2026
10 of 16 checks passed
@HaochenYuan

Copy link
Copy Markdown
Contributor

@HaochenYuan, #2898 recently changed some assumptions with FlashAttention which broke CI, this is a fix, could you confirm/review that these changes don't affect your use-case? Thanks!

Thanks for the fix, and it won't affect my use case~

KshitijLakhani pushed a commit that referenced this pull request Jul 30, 2026
…P tests (#3269)

* [PyTorch] Honor requested FA padding in CP tests

The CP test runner creates inter-sequence padding when the FlashAttention padding case is requested, but it unconditionally told DPA that FlashAttention THD inputs had no padding. That mismatch left CP backward padding uninitialized.\n\nDerive the explicit padding state from the same condition used to generate the inputs and reuse it for both reference and CP calls, preserving the sync-free CUDA-graph path for non-padding cases.

Signed-off-by: Sudhakar Singh <sudhakars@nvidia.com>

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

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

---------

Signed-off-by: Sudhakar Singh <sudhakars@nvidia.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.

5 participants