Skip to content

Add THD-format per-rank batch fetch with padding mask for sequence packing - #5904

Merged
ilml merged 2 commits into
NVIDIA:pull-request/5903from
ilml:split/3386-04-thd-batch-fetch
Aug 17, 2026
Merged

Add THD-format per-rank batch fetch with padding mask for sequence packing#5904
ilml merged 2 commits into
NVIDIA:pull-request/5903from
ilml:split/3386-04-thd-batch-fetch

Conversation

@ilml

@ilml ilml commented Jul 20, 2026

Copy link
Copy Markdown
Contributor
  • I, the PR author, have personally reviewed every line of this PR.

What does this PR do?

Adds get_batch_on_this_rank_for_sequence_packing (TP/PP broadcast of packed microbatches, PackedSeqParams construction, CP partitioning), the THD padding-mask helpers, and the get_thd_partitioned_indices TE wrapper, with unit tests.

Part 04/10 of splitting #3386 (Add E2E support for THD format; dev-branch PR #2924). Original changes by @xiaoyao0115 in #3386 — split into functionally self-contained PRs to ease review. Hard dependencies (must merge first): #5903.

Split series (#3386)

Part PR Hard deps (merge first)
01 #5901
02 #5902
03 #5903
04 #5904 #5903
05 #5905 #5904
06 #5906 #5902, #5903
07 #5907 #5902, #5904, #5905
08 #5908
09 #5909 #5906, #5908
10 #5910 #5907, #5909

Branches are stacked linearly (each on the previous) so every PR shows a clean own-diff once its base is retargeted to the copy-pr-bot pull-request/<parent> ref; until then the Files-changed view of a stacked PR includes its ancestors — its own change is the last commit.
Stacked on #5903.

Review notes

  • The parametrized test_get_batch_on_this_rank_for_sequence_packing needs the 8-GPU torchrun CI harness and TE >= 2.9.0 for the packing paths.

Issue tracking

Linked issue: Related to #3386

Contribution process

Pre-checks

  • I have added relevant unit tests
  • I have added relevant functional tests
  • I have added proper typing to my code Typing guidelines
  • I have added relevant documentation
  • I have run the autoformatter.sh on my PR

🤖 Generated with Claude Code

@copy-pr-bot

copy-pr-bot Bot commented Jul 20, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Comment thread megatron/core/extensions/transformer_engine.py Outdated
Comment thread megatron/core/datasets/data_schedule.py
@svcnvidia-nemo-ci svcnvidia-nemo-ci added the Final Review PR is in the "final review" stage label Jul 30, 2026
@ilml
ilml force-pushed the split/3386-04-thd-batch-fetch branch from 12334c0 to 1cc3a9e Compare July 30, 2026 21:19
@ilml
ilml force-pushed the split/3386-04-thd-batch-fetch branch 2 times, most recently from e8e5f20 to ce62c99 Compare August 6, 2026 16:44
@ilml
ilml force-pushed the split/3386-04-thd-batch-fetch branch from ce62c99 to 48fecb2 Compare August 10, 2026 22:04
ilml and others added 2 commits August 10, 2026 22:05
…cking

Split 4/10 from NVIDIA#3386 (sequence packing / THD E2E support). Adds
get_batch_on_this_rank_for_sequence_packing (TP/PP broadcast,
PackedSeqParams construction, CP partitioning via the new
get_thd_partitioned_indices TE wrapper) plus THD padding-mask helpers
and their unit tests.

Original changes by @xiaoyao0115 in NVIDIA#3386.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: ilml <tolong@nvidia.com>
Per review feedback on NVIDIA#5904:

- Remove get_thd_partitioned_indices from the TE extensions module and
  call tex.thd_get_partitioned_indices directly, matching the existing
  call site in megatron/core/utils.py and the other tex users
  (ssm/mamba_context_parallel.py, models/mimo, models/multimodal). The
  is_te_min_version("1.10.0") gate was unreachable -- the supported TE
  floor is 2.16.0 (LTS) / 2.18.0 (dev) -- and it misreported a missing
  TE as an outdated one, since is_te_min_version returns False when TE
  is absent. The guarded 'tex is not None' import also keeps
  megatron/core/datasets free of an eager dependency on the TE
  extensions module.

- Reuse data_schedule_utils.broadcast_scalars for the two receive-buffer
  shapes instead of hand-rolling them: two collectives and two device
  syncs collapse into one of each.

- Drop local_cp_size=None / cp_group=None from the PackedSeqParams call;
  both are already the dataclass defaults.

Original changes by @xiaoyao0115 in NVIDIA#3386.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: ilml <tolong@nvidia.com>
@ilml
ilml force-pushed the split/3386-04-thd-batch-fetch branch from 48fecb2 to 956a101 Compare August 10, 2026 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

complexity: high Final Review PR is in the "final review" stage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants