Skip to content

Integrate sequence-packing scheduler into training loop and wire varlen dataset into GPT pretraining - #6742

Open
ilml wants to merge 4 commits into
NVIDIA:mainfrom
ilml:split/3386-training-r2
Open

Integrate sequence-packing scheduler into training loop and wire varlen dataset into GPT pretraining#6742
ilml wants to merge 4 commits into
NVIDIA:mainfrom
ilml:split/3386-training-r2

Conversation

@ilml

@ilml ilml commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Integrates the sequence-packing scheduler into train_step/evaluate and wires the varlen dataset into the pretraining CLI, dataloader, and dataset provider (--use-varlen-dataset) — for both pretrain_gpt and pretrain_hybrid (per review: HybridModel.forward already accepts packed_seq_params/padding_mask, so the hybrid entry point gets the same batch-fetch, forward_step, and provider wiring; note hybrid THD wiring goes beyond the original #3386, which only wired GPT). Final PR of the #3386 split series tracked in the Dynamic Context Parallelism project — after this, the full THD/sequence-packing E2E from #3386 is landed. Original changes by @xiaoyao0115 in #3386.

Note: Replaces #6686, which was auto-closed when its base mirror ref was deleted. All of @jaredcasper's review comments on #6686 are addressed here: no more getattr defensiveness in data_samplers.py (direct attribute access; the dead use_vanilla_collate_fn read is gone), the seqlen-stats accumulator block stays at its original location in training.py instead of moving, and _add_varlen_dataset_args matches the file's argument formatting. The CLI-selector checks remain in validate_args because --use-varlen-dataset/--sft/--mock-data are not core config fields (the field-owned checks moved to ModelParallelConfig.__post_init__ via #5902).

Series status

Remaining PRs of the #3386 split, tracked in the Dynamic Context Parallelism project. All PRs target main — no stacked bases. Reviews run in parallel; merges go in order:

Order PR Content
1 #5902 config field, CLI arg, validation
2 #6734 mock SFT + varlen datasets
3 this PR (#6742) training-loop + varlen wiring (pretrain_gpt + pretrain_hybrid)

#6734 carries a temporary copy of #5902's commit, and #6742 carries #6734's commits (review its last three commits) — the duplicates drop out automatically as parents merge and the branches are rebased.

Merged so far: #5901, #5903, #6625, #5908, #6684, #5902, #6734. This is the final PR of the series.

Diff note: rebased onto main after #6734 merged — Files-changed now shows exactly this PR's three commits (training-loop integration, varlen wiring, pretrain_hybrid wiring).

Contribution process

  • Draft PR per contributing guidelines
  • Commits signed off (DCO)

@ilml

ilml commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test b9a71e3

@copy-pr-bot

copy-pr-bot Bot commented Aug 21, 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.

@ilml

ilml commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test c467af1

@ilml
ilml changed the base branch from pull-request/6734 to main August 21, 2026 05:03
@ilml
ilml marked this pull request as ready for review August 21, 2026 05:04
@ilml
ilml requested review from a team as code owners August 21, 2026 05:04
data_parallel_size=mpu.get_data_parallel_world_size())
elif args.dataloader_type == 'single':
if args.hybrid_context_parallel:
if args.hybrid_context_parallel and args.sequence_packing_scheduler is None:

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.

not in this PR but we should change everything to "dynamic" now?

@ilml

ilml commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 095c361

@ilml

ilml commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 0a0f9e3

@ilml

ilml commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 4ccffde

@ilml

ilml commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test c7208e1

@ilml

ilml commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test ac3d429

@ilml

ilml commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 9268336

ilml and others added 3 commits August 27, 2026 16:14
…aining

Split 7/10 from NVIDIA#3386 (sequence packing / THD E2E support). Wires
wrap_data_iterator into train_step/evaluate with dynamic
num_microbatches, adds global seqlen-stats plumbing for FLOPs
accounting, gates the HybridCP sampler, and restructures
pretrain_gpt.forward_step for packed (THD) batches with padding mask.

Original changes by @xiaoyao0115 in NVIDIA#3386.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: ilml <tolong@nvidia.com>
…vider

Split 10/10 from NVIDIA#3386 (sequence packing / THD E2E support). Adds
--use-varlen-dataset / --varlen-sbhd-validation /
--varlen-mock-dataset-config-json, their validation (auto-selecting the
dp_balanced scheduler), the dataset-provider wiring in pretrain_gpt,
and the remaining dataloader/unpack 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>
…in_hybrid

Per review: mirror the pretrain_gpt integration for HybridModel, whose
forward already accepts packed_seq_params and padding_mask (packed THD
batches are exercised today by hybrid SFT).

- get_batch takes the get_batch_on_this_rank_for_sequence_packing path
  when --sequence-packing-scheduler is set, same as pretrain_gpt.
- forward_step handles the scheduler's 6/7-tuple batches and threads
  padding_mask through to the model; the legacy SFT/inter-document path
  is unchanged apart from the enclosing tuple dispatch.
- The dataset provider builds VarlenDataset/MockVarlenDataset under
  --use-varlen-dataset and passes the varlen config fields through.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: ilml <tolong@nvidia.com>
@ilml
ilml force-pushed the split/3386-training-r2 branch from 9268336 to 97e73af Compare August 27, 2026 16:14
@ilml

ilml commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 97e73af09b03256de883166a86590fa77e73ec99

@copy-pr-bot

copy-pr-bot Bot commented Aug 27, 2026

Copy link
Copy Markdown

/ok to test 97e73af09b03256de883166a86590fa77e73ec99

@ilml, there was an error processing your request: E2

See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/2/

@svcnvidia-nemo-ci svcnvidia-nemo-ci added the Approved All necessary approvals have been made label Aug 27, 2026
@ilml
ilml enabled auto-merge August 27, 2026 18:51
@ilml

ilml commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 97e73af

@ilml

ilml commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 992c2b7

The repo black config skips magic trailing commas; apply exactly the
hunk the lint job suggests.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: ilml <tolong@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved All necessary approvals have been made complexity: high

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants