feat(data): scalable canonical-grid reads for MegatronMIMO data loading - #5525
Open
going-song wants to merge 1 commit into
Open
feat(data): scalable canonical-grid reads for MegatronMIMO data loading#5525going-song wants to merge 1 commit into
going-song wants to merge 1 commit into
Conversation
going-song
commented
Aug 12, 2026
| if scalable_dp: | ||
| module_parallelism = megatron_mimo_cfg.module_parallelisms[my_module] | ||
| if module_parallelism.expert_tensor_parallel_size != 1: | ||
| raise NotImplementedError( |
Contributor
Author
There was a problem hiding this comment.
The default slicing path relies on the same assumption — should this live in MegatronMIMOParallelismConfig validation instead?
going-song
commented
Aug 12, 2026
| # Truncate to whole global micro-batches: the flat samplers round their active range at | ||
| # per-group granularity, which diverges per group for a non-multiple dataset size (the | ||
| # cyclic data_sharding=False stride would give groups unequal window counts). | ||
| total_samples = (len(dataset) // micro_batch_size) * micro_batch_size |
Contributor
Author
There was a problem hiding this comment.
Prevents unequal per-group window counts on ragged dataset sizes (cyclic + data_sharding=False); regression-tested.
going-song
commented
Aug 12, 2026
| dataloader_type="single", | ||
| trust_remote_code=False, | ||
| do_validation=do_validation, | ||
| pack_sequences_in_batch=False, |
Contributor
Author
There was a problem hiding this comment.
This test has been failing on main since #5131 (missing pack_sequences_in_batch); adding both kwargs.
liding-nv
self-requested a review
August 15, 2026 03:16
Contributor
|
/ok to test 958575f |
By default every MegatronMIMO data-loading rank reads the full global micro-batch and forward_step slices out its module-DP shard, so per-rank read and preprocessing cost scales with the DP degree. This adds an opt-in dataset switch, `megatron_mimo_scalable_dp`, that shards reads at the sampler instead and skips the forward-step slice. Reads are sharded on a canonical grid: the LCM of the module DP sizes. Each rank covers `grid // dp` consecutive canonical groups and concatenates one flat Megatron sampler per group, all built with the same `(micro_batch // grid, grid)` geometry and the global consumed_samples. Group streams depend only on module-independent inputs, so every module materializes the identical ordered global micro-batch under both "single" and the default "cyclic" sampler, keeping the BridgeCommunicator's positional batch-dim routing aligned. (Sharding by each module's own DP - the original mechanism - only aligns under "single"; "cyclic" became the direct-HF-SFT default in NVIDIA-NeMo#5048.) Group geometries are truncated to whole global micro-batches so ragged dataset sizes cannot desynchronize the groups' window counts. Split (2/3) of NVIDIA-NeMo#4608 as agreed in NVIDIA-NeMo#4609. Original work by Chanwoo Park. Validated: unit tests for cross-geometry alignment, resume across epoch boundaries, and non-multiple dataset sizes; live PP=2 runs (language tp1/pp2/dp2 + images dp1): cyclic+scalable completes where module-DP sharding crashed on embedding-count mismatch, single scalable matches non-scalable losses within bf16 noise, and the default path matches prior losses within run-to-run noise. Signed-off-by: kayeon.song <kayeon.song@navercorp.com> Signed-off-by: Yoonsik Kim <yoonsik.kim90@navercorp.com> Signed-off-by: Chanwoo Park <chanwoo.park98@navercorp.com> Co-authored-by: Yoonsik Kim <yoonsik.kim90@navercorp.com> Co-authored-by: Chanwoo Park <chanwoo.park98@navercorp.com>
going-song
force-pushed
the
feat/megatron-mimo-canonical-reads
branch
from
August 18, 2026 11:33
958575f to
2f6f339
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds opt-in scalable data-parallel reads to MegatronMIMO. By default every data-loading
rank reads the full global micro-batch and
forward_stepslices out its module-DP shard, soper-rank read and preprocessing cost scales with the DP degree. With
dataset.megatron_mimo_scalable_dpeach rank's sampler reads only its shard and the slice isskipped:
Second of three PRs splitting #4608 (original work by @sailor1493, credited
as co-author and signer), per the maintainers' split request in #4609.
Part 1 merged as #5131.
Why a canonical grid (mechanism deviation from #4608)
#4608 sharded each module's loader by its own module-DP
(rank, size). A sampler's shardassignment is a function of
(micro_batch_size, data_parallel_size), so under"cyclic"—the direct-HF-SFT default since #5048 — modules with different DP sizes read different
sample sets, while the
BridgeCommunicatorroutes modality embeddings positionally alongthe batch dim. We hit this live as an embedding-count crash in
align_embeddings_by_token_positions; with uniform image sizes it would silently train onmispaired image-text.
Here all loaders shard on one canonical grid — the LCM of the module DP sizes. Every
group sampler uses the identical
(micro_batch // grid, grid)geometry and globalconsumed_samples; a rank whose module has DP sizedconcatenates the windows of itsgrid // dconsecutive groups (CanonicalGroupBatchSampler). Group streams aremodule-independent, so every module materializes the identical ordered micro-batch under
"single"and"cyclic"; for"single"the stream is bit-identical to the defaultpath's slice. Geometries are truncated to whole micro-batches so ragged dataset sizes cannot
desynchronize the groups. ("Canonical group" is the vocabulary of #4608's reordering
machinery, which ③ will port.)
Configuration
"single"and"cyclic"are supported;"batch"/"external"anddrop_last=Falsearerejected in
validate()and again at loader build. Off by default — with the switch unset, thesampler parameters, loader arguments and slicing behavior are identical to main.
Notes for reviewers
expert_tensor_parallel_size > 1(unvalidated combination). Thedefault slicing path shares the same assumption — happy to hoist the guard into
MegatronMIMOParallelismConfigvalidation if preferred.variable_seq_lengths=True, so the schedule'smicro_batch_sizeis shape-inert and the LR/consumed accounting already uses the fullmicro-batch in both modes (feat(data): intra-microbatch reordering for MegatronMIMO (+ sequence packing, scalable DP) #4608's schedule/LR adjustment is unnecessary here).
"cyclic", scalable on/off are two different valid epoch shuffles (per-iterationlosses not comparable);
"single"is order-identical.Testing
258 unit tests passing locally, including the cross-geometry alignment invariant (a rank
covering groups {0,1} reads exactly the concatenation of the single-group ranks' windows, for
"cyclic"in bothdata_shardingmodes) and a library-path end-to-end test with DP {2,3}(grid = 6). Live PP=2 runs (5 GPUs of an 8xA100 node,
language=tp1,pp2,dp2+images=dp1, Qwen3.5-0.8B randominit, cord_v2, mbs 4, 10 iters):
cyclic+scalable completes 10/10 where module-DP shardingcrashes;
singlescalable matches the default losses within bf16 noise (max |diff| 0.0043);the default path matches prior losses within run-to-run noise (max |diff| 0.0024); packing on/off parity under scalable ~0.001.
(One drive-by repair: the example-config test's arg namespace was already failing on main —
#5131 added
pack_sequences_in_batchto_build_dataset_configwithout updating it — so thisPR adds both missing kwargs there.)
Known limitations
ETP > 1 and
"batch"/"external"loaders are rejected for scalable reads; MegatronMIMO stillcannot run CP (pre-existing); checkpoint resume is unit-tested but not yet exercised in a live
run, and multi-node validation is pending, as with #4608.
Signed-off-by: kayeon.song kayeon.song@navercorp.com
Signed-off-by: Yoonsik Kim yoonsik.kim90@navercorp.com
Signed-off-by: Chanwoo Park chanwoo.park98@navercorp.com