Skip to content

[PyTorch] Extend no-load-balance CP to the a2a comm type - #3530

Open
Rudin6 wants to merge 2 commits into
NVIDIA:mainfrom
Rudin6:cp-a2a-no-load-balance
Open

Rudin6 wants to merge 2 commits into
NVIDIA:mainfrom
Rudin6:cp-a2a-no-load-balance

Conversation

@Rudin6

@Rudin6 Rudin6 commented Sep 16, 2026

Copy link
Copy Markdown

Flat (no-load-balance) sharding gives CP rank r the contiguous global token range [r * s_local, (r + 1) * s_local). For cp_comm_type='a2a' the exchange already restores the full sequence on every rank before attention, so the dual-chunk reordering is a no-op that costs a full extra copy of q, k and v.

a2a never divides cu_seqlens or max_seqlen: after the exchange each rank holds the whole sequence and only its own head slice, so the sequence metadata, the mask and the attention math are identical to cp_size = 1. Every attn_mask_type and window_size therefore stays valid, and THD needs no new per-step metadata.

Skipping the reorder also removes its view(cp_size * 2, ...), which is the only reason the local sequence length had to be even. The per-sequence divisibility requirement drops from 2 * cp_size to cp_size; for THD no individual sequence needs padding at all, only the packed total.

Measured on 8xH200 for one before_attn exchange of q+k+v (bshd, cp=8, h=40, d=128), max over ranks: -12.0% at s=8192, -29.5% at s=32768 and -31.7% at s=75600. The gain grows with payload because the removed index_select is bandwidth bound.

Scope matches the existing all_gather strategy: FP8 and CUDA graph capture stay rejected, and a2a+p2p is not covered. The p2p path is unchanged.

Tests add flat a2a coverage over bshd, sbhd and THD, including a config whose sequence length is divisible by cp_size but not by 2 * cp_size, and a causal config. All 9 pass; dual-chunk a2a on the same configs and the existing all_gather no-load-balance tests still pass.

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

Flat (no-load-balance) sharding gives CP rank r the contiguous global token
range [r * s_local, (r + 1) * s_local). For cp_comm_type='a2a' the exchange
already restores the full sequence on every rank before attention, so the
dual-chunk reordering is a no-op that costs a full extra copy of q, k and v.

a2a never divides cu_seqlens or max_seqlen: after the exchange each rank holds
the whole sequence and only its own head slice, so the sequence metadata, the
mask and the attention math are identical to cp_size = 1. Every attn_mask_type
and window_size therefore stays valid, and THD needs no new per-step metadata.

Skipping the reorder also removes its view(cp_size * 2, ...), which is the only
reason the local sequence length had to be even. The per-sequence divisibility
requirement drops from 2 * cp_size to cp_size; for THD no individual sequence
needs padding at all, only the packed total.

Measured on 8xH200 for one before_attn exchange of q+k+v (bshd, cp=8, h=40,
d=128), max over ranks: -12.0% at s=8192, -29.5% at s=32768 and -31.7% at
s=75600. The gain grows with payload because the removed index_select is
bandwidth bound.

Scope matches the existing all_gather strategy: FP8 and CUDA graph capture stay
rejected, and a2a+p2p is not covered. The p2p path is unchanged.

Tests add flat a2a coverage over bshd, sbhd and THD, including a config whose
sequence length is divisible by cp_size but not by 2 * cp_size, and a causal
config. All 9 pass; dual-chunk a2a on the same configs and the existing
all_gather no-load-balance tests still pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Rudin6 <41809200+Rudin6@users.noreply.github.com>
@Rudin6
Rudin6 requested a review from cyanguwa as a code owner September 16, 2026 16:49
@github-actions github-actions Bot added the community-contribution PRs from external contributor outside the core maintainers, representing community-driven work. label Sep 16, 2026
@greptile-apps

greptile-apps Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The production changes appear safe to merge, with one non-blocking test-harness limitation still outstanding from the previous review.

Findings

  1. P2 Flat bias tests crash

Summary

This PR extends no-load-balance context parallelism to the A2A communication path.

  • Skips dual-chunk sequence reordering when tokens are already flat-sharded in global sequence order.
  • Relaxes the local even-sequence-length restriction for flat A2A operation.
  • Preserves global THD sequence metadata after the exchange reconstructs the full packed sequence.
  • Adds fused-attention coverage for bshd, sbhd, and thd, including causal attention and sequence lengths not divisible by twice the CP size.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[Flat local token shard] --> B[A2A exchange]
    B --> C[Full global sequence<br/>local head shard]
    C --> D[Attention]
    D --> E[Split sequence by CP rank]
    E --> F[A2A exchange]
    F --> G[Local token shard<br/>full heads]
Loading

Reviews (2) · Last reviewed commit: "Merge branch 'main' into cp-a2a-no-load-..."

Comment on lines +516 to +529
if no_load_balance:
# Flat shard: rank r owns the contiguous global range
# [r * s_local, (r + 1) * s_local). No chunk swap, so no 2 * cp factor.
# clone(), not contiguous(): for sbhd the narrowed slice is already
# contiguous, so contiguous() is a no-op and leaves a nonzero
# storage_offset, which get_qkv_layout rejects.
q_, k_, v_, dout_ = [
x.narrow(
seq_dim,
rank * (x.shape[seq_dim] // world_size),
x.shape[seq_dim] // world_size,
).clone()
for x in [q_, k_, v_, dout_]
]

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.

P2 Flat bias tests crash

This flat bshd/sbhd branch does not initialize seq_idx, but the later attention-bias setup still reads it and assumes a 2 * world_size dual-chunk layout. As a result, any flat-a2a test configured with attention bias will fail with an unbound-variable error instead of validating the feature. The new configurations all use no_bias, so they do not cover this path.

This branch has not been deployed

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

Labels

attention 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