Skip to content

fix(te): pass pad_between_seqs explicitly for tail-padded THD under CP - #25

Merged
JackRao123 merged 1 commit into
trainers-mainfrom
jackrao/lps-1063-thd-cp-pad-between-seqs
Aug 8, 2026
Merged

fix(te): pass pad_between_seqs explicitly for tail-padded THD under CP#25
JackRao123 merged 1 commit into
trainers-mainfrom
jackrao/lps-1063-thd-cp-pad-between-seqs

Conversation

@JackRao123

@JackRao123 JackRao123 commented Aug 8, 2026

Copy link
Copy Markdown

The bug

Packed (THD) batches describe sequence boundaries with two lists of running totals:

  • cu_seqlens — counting real tokens only. One 698-token sequence → [0, 698].
  • cu_seqlens_padded — including padding. Padded to 704 → [0, 704].
  • pad_between_seqs — a flag telling TE the two lists differ. True → TE computes each CP rank's token counts exactly. False → shortcut: divide the totals by the number of ranks.

TE guesses the flag by comparing the lists without their last entry ([:-1]). Padding at the end of the batch only shows in the last entry — so for our batches TE always guesses False and takes the shortcut, which miscounts whenever the real length doesn't divide evenly:

13 real tokens (t0–t12), padded to 16. CP=2, zigzag chunks of 4:
rank 0 holds chunks 0+3, rank 1 holds chunks 1+2.

           rank 0                        rank 1
tokens:  t0 t1 t2 t3 | t12 p13 p14 p15   t4 t5 t6 t7 | t8 t9 t10 t11
truth:   5 of 8 real                     8 of 8 real

TE's guess (pad_between_seqs=False):  13 // 2 = 6 real, on both ranks.

  rank 0: p13 (padding) counted as real → reads uninitialized memory
          → different answer every run
  rank 1: t10, t11 (real) counted as padding → dropped from attention
          → silently wrong, same every run

The first failure mode is what broke the Nemotron nightly ckpt-roundtrip gate; the second ships silently in every CP>1 THD training run.

The fix

Don't let TE guess: we have both lists at the call site, so compare them fully (last entry included) and pass the flag explicitly. Scoped to CP because that's the only place the shortcut is wrong — setting the flag elsewhere would needlessly switch attention kernels.

Validation

  • Standalone 4-GPU repro: bitwise-deterministic fwd+bwd, 100/100 runs × 3 seeds × CP{2,4}; CP4 matches CP1 ground truth to bf16 rounding.
  • Full trainer stack (TP2×CP4): 10/10 bit-identical forwards vs a nondeterministic control.
  • Full record: trainers repo, experiment_artefacts/lps1073/ (LPS-1073, split from the original LPS-1063).

Downstream bumps: basetenlabs/Megatron-Bridge#31 → basetenlabs/trainers#994.

🤖 Generated with Claude Code

@linear

linear Bot commented Aug 8, 2026

Copy link
Copy Markdown

LPS-1063

@JackRao123
JackRao123 force-pushed the jackrao/lps-1063-thd-cp-pad-between-seqs branch from e43ae91 to 6e8e281 Compare August 8, 2026 22:40
JackRao123 added a commit to basetenlabs/Megatron-Bridge that referenced this pull request Aug 8, 2026
…D under CP)

Picks up basetenlabs/Megatron-LM#25 (e43ae913f): TEDotProductAttention now
passes the tail-inclusive pad_between_seqs answer explicitly under context
parallelism, engaging TE's exact get_cu_seqlens_on_cp_rank path. Fixes the
LPS-1063 nondeterministic forward/gradients and silent boundary-row
mis-attention caused by TE's cu_seqlens[:-1] auto-detect
(NVIDIA/TransformerEngine#3331).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Released TE's pad_between_seqs auto-detect ignores padding after the last
sequence; under context parallelism that silently corrupts chunk-boundary
rows (nondeterministic forward/gradients + wrong attention). Compute the
tail-inclusive answer at the call site and pass it explicitly.

Details: #25.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@JackRao123
JackRao123 force-pushed the jackrao/lps-1063-thd-cp-pad-between-seqs branch from 6e8e281 to 2919b7d Compare August 8, 2026 22:47
JackRao123 added a commit to basetenlabs/Megatron-Bridge that referenced this pull request Aug 8, 2026
…D under CP)

Picks up basetenlabs/Megatron-LM#25 (e43ae913f): TEDotProductAttention now
passes the tail-inclusive pad_between_seqs answer explicitly under context
parallelism, engaging TE's exact get_cu_seqlens_on_cp_rank path. Fixes the
LPS-1063 nondeterministic forward/gradients and silent boundary-row
mis-attention caused by TE's cu_seqlens[:-1] auto-detect
(NVIDIA/TransformerEngine#3331).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
qkv_format = packed_seq_kwargs.get('qkv_format', self.qkv_format)

# Released TE mis-detects tail-only padding as "no padding", which
# under context parallelism silently corrupts chunk-boundary rows.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

lets clean up these comments # slop

@JackRao123
JackRao123 merged commit 57efae0 into trainers-main Aug 8, 2026
1 check passed
JackRao123 added a commit to basetenlabs/Megatron-Bridge that referenced this pull request Aug 8, 2026
…D under CP) (#31)

* build: bump Megatron-LM (explicit pad_between_seqs for tail-padded THD under CP)

Picks up basetenlabs/Megatron-LM#25 (e43ae913f): TEDotProductAttention now
passes the tail-inclusive pad_between_seqs answer explicitly under context
parallelism, engaging TE's exact get_cu_seqlens_on_cp_rank path. Fixes the
LPS-1063 nondeterministic forward/gradients and silent boundary-row
mis-attention caused by TE's cu_seqlens[:-1] auto-detect
(NVIDIA/TransformerEngine#3331).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* build: pin Megatron-LM to merged trainers-main HEAD (57efae08)

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
jerryhong21 pushed a commit that referenced this pull request Aug 20, 2026
#25)

Released TE's pad_between_seqs auto-detect ignores padding after the last
sequence; under context parallelism that silently corrupts chunk-boundary
rows (nondeterministic forward/gradients + wrong attention). Compute the
tail-inclusive answer at the call site and pass it explicitly.

Details: #25.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
jerryhong21 pushed a commit that referenced this pull request Aug 20, 2026
#25)

Released TE's pad_between_seqs auto-detect ignores padding after the last
sequence; under context parallelism that silently corrupts chunk-boundary
rows (nondeterministic forward/gradients + wrong attention). Compute the
tail-inclusive answer at the call site and pass it explicitly.

Details: #25.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
jerryhong21 pushed a commit that referenced this pull request Aug 21, 2026
#25)

Released TE's pad_between_seqs auto-detect ignores padding after the last
sequence; under context parallelism that silently corrupts chunk-boundary
rows (nondeterministic forward/gradients + wrong attention). Compute the
tail-inclusive answer at the call site and pass it explicitly.

Details: #25.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.

2 participants