Repository navigation
chore: AUT-2598 nightly sync main into dev (2026-10-20) - #8402
Conversation
…IDIA#7748) Signed-off-by: Haoran Zhang <haoranz@nvidia.com> Co-authored-by: Shiqing Fan <shiqingf@nvidia.com>
Signed-off-by: Guihong Li <guihongl@nvidia.com>
…#8171) Signed-off-by: Keshav Santhanam <ksanthanam@nvidia.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Philip Petrakian <ppetrakian@nvidia.com>
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
…ns (NVIDIA#8030) Signed-off-by: adistomar <adityatomar2604@gmail.com>
Signed-off-by: adistomar <adityatomar2604@gmail.com>
…A#8144) Signed-off-by: Siddhartha Raman S <sraman@nvidia.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ting (NVIDIA#7709) Signed-off-by: ilml <tolong@nvidia.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…DIA#8148) Signed-off-by: Ajay Balasa <abalasa@nvidia.com>
Signed-off-by: Ajay Balasa <abalasa@nvidia.com>
…IA#8193) Signed-off-by: svcnemo-autobot <svcnemo-autobot@nvidia.com>
Signed-off-by: Alok Tripathy <atripathy@nvidia.com>
Signed-off-by: Lawrence McAfee <lmcafee@nvidia.com>
Signed-off-by: Siddhartha Raman S <sraman@nvidia.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…p64 to match training (NVIDIA#8195) Signed-off-by: YigongQin <yigongq@nvidia.com>
…or the indexer and MLA attention (NVIDIA#7522) Signed-off-by: Hollow Man <hollowman@opensuse.org>
…A#6859) Signed-off-by: Rui Zhu <rui.zhu.rz399@yale.edu> Co-authored-by: Charlie Truong <chtruong@nvidia.com>
…7728) Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
Signed-off-by: Ajay Balasa <abalasa@nvidia.com>
Signed-off-by: Anil Thomas <anthomas@nvidia.com> Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Ajay Balasa <abalasa@nvidia.com>
…IA#7768) Signed-off-by: Jiangfei Duan <jiangfeid@nvidia.com>
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
Signed-off-by: Haoran Zhang <haoranz@nvidia.com>
…A#7774) Signed-off-by: Jiangfei Duan <jiangfeid@nvidia.com>
NVIDIA#8238) Signed-off-by: Ajay Balasa <abalasa@nvidia.com>
… after eviction (NVIDIA#8150) Signed-off-by: wdykas <wdykas@nvidia.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
/review model=claude mode=strict |
|
/ok to test 0b27aa7 |
|
/review mode=strict model=claude |
svcnvidia-nemo-ci
left a comment
There was a problem hiding this comment.
Strict review: nightly sync main → dev (2026-10-20)
Findings: 0 CRITICAL · 0 IMPORTANT · 1 SUGGESTION
Overall risk: low for the parts I checked, which include all of the repair commit's source changes. The rest of this 981-file sync was not checked, so I am leaving a comment rather than an approval.
Finding
- [SUGGESTION Simplification] The repair commit added a
precomputed_indicesparameter toInferenceTopKRouter._compiled_topk_routingin router.py. Nothing ever passes it a value. The only caller passesprecomputed_indicesonly when it is set, and in exactly that case it uses the uncompiledtopk_routing_with_score_functioninstead. The base code already had this caller unchanged, so the parameter does not fix a mismatch. Details are in the inline comment.
What I checked and found no issue with
MHCCheckpointManagernow subclassesCheckpointWithoutOutputManager(random.py). This matches the internals from main #8334. The RNG helpers it relies on (_get_all_rng_states,_set_all_rng_states,_fork_rng) exist and are also used byRecomputeSegmentin model_chunk_schedule_plan.py._HybridEPManager.setup_metadata(token_dispatcher.py). The token budget is handled correctly in all three cases:- The rank-capacity branch sets the budget.
- The branch with no capacity setting resets
num_permuted_tokenstoNone. - drop-and-pad still overrides it with the capacity value, and combine keeps the static budgets.
- Startup ownership after main #8272.
initialize_megatronnow callsinitialize_runtime_services, which resolves the tokenizer vocab size. That service setup is not safe to run twice. Every changed entry point (BERT, MIMO, inference examples, detxoify, and the dev-only multimodal_dev) correctly dropped its own explicit call.pretrain(..., *, build_tokenizer=...)exists (training.py), sopretrain_mimo'sbuild_tokenizer=Falseis valid. - MIMO changes.
DistributedInitConfighasalign_grad_reduce(defaultTrue) andhigh_priority_stream_groups, which grad_sync.py and pretrain_mimo.py now read.get_data_lane_rankreadspg_collection.dp_gtp_remat, which topology.py populates for every grid.
- Multimodal dataloader resume. The new
(args, iteration)signature is used consistently bydataloader_providerand its unit test. - CI.
- The new reusable _unit-tests.yml workflow runs the full bucket on GPU unless selective mode is
enforce. - The coverage gate requires an empty-selection plan for every bucket.
- Removing A100 from the GitLab stages is consistent across configure, run, cleanup, notify and publish.
- The gb200 unit-test recipe now has a single
test_gated_delta_net.pybucket.
- The new reusable _unit-tests.yml workflow runs the full bucket on GPU unless selective mode is
- Router quantile balancing. The router implements both the
micro_batchandglobal_batchestimation scopes, so having the virtual-expert tests selectmicro_batchonly changes the tests.
What this review did not cover
I prioritised source code over config and generated files, following the strict-mode guidance for PRs with more than 50 files. I inspected the repair-commit sources, the inlined CI/docs/examples hunks, and the cross-component paths they touch (training startup, the MIMO topology and helpers, and the MoE router and dispatcher).
I did not individually inspect:
- the large omitted core diffs, such as MFSDP v2, disaggregated inference, GTP sequence sharding and dist-checkpointing;
- most of the about 500 golden-value and recipe files.
Those rely on the author's reported local runs and pending CI. reviewed_files lists only what I inspected.
Review generated by claude. This was a findings-only review. Request a full review with /review reset=true.
| @@ -1417,6 +1437,7 @@ def _compiled_topk_routing( | |||
| fused, | |||
| router_replay, | |||
| dense_output, | |||
| precomputed_indices=None, | |||
There was a problem hiding this comment.
[SUGGESTION Simplification] Strict mode checks for unused arguments: the new precomputed_indices=None parameter of InferenceTopKRouter._compiled_topk_routing is never given a value.
Problem: The only caller is _forward (L1476-L1500). It adds precomputed_indices to routing_kwargs only when self.qb_beta is not None. In exactly that case it picks the uncompiled topk_routing_with_score_function (routing = topk_routing_with_score_function if precomputed_indices is not None or is_batch_invariant_mode_enabled() else self._compiled_topk_routing). So when _compiled_topk_routing runs, routing_kwargs == {} and precomputed_indices is always None.
The base caller is identical (base L1454-L1478) and never passed the argument to the compiled function. The commit message's reason ("matching the base router call site") therefore does not correspond to an actual mismatch. The dead parameter also suggests the quantile-balancing path is compiled when it is not.
Fix (smallest change): remove the parameter and the forwarded keyword so the compiled helper matches its real contract.
router_replay,
dense_output,
- precomputed_indices=None,
):
return topk_routing_with_score_function(
...
dense_output=dense_output,
- precomputed_indices=precomputed_indices,
)If you actually want quantile-balanced (qb_beta) inference routing to run through torch.compile, make that change on purpose in _forward's routing choice instead of leaving a parameter that is never used. That alternative changes behaviour and compilation (an optional tensor argument can trigger recompiles), so it needs its own validation.
Behaviour to keep:
- With
qb_betaset,_forwardkeeps calling the uncompiledtopk_routing_with_score_function(..., precomputed_indices=...). - Without
qb_betaand outside batch-invariant mode,_forwardkeeps calling_compiled_topk_routingwith the same 11 arguments it uses today. - In batch-invariant mode, the uncompiled path stays in use.
- Hash-layer routing is unaffected.
How to verify:
- Static check: after the change, confirm that nothing passes
precomputed_indicesto_compiled_topk_routing. I found no tests that reference_compiled_topk_routing. - Tests to run (I could not run them):
tests/unit_tests/transformer/moe/test_routers.pyandtests/unit_tests/inference/test_moe_dispatching_and_routing.py. They should still pass, in particular the inference-router and batch-invariant training/inference parity cases (assert_close(..., atol=0, rtol=0)).
|
Reply to review 5602919574 / inline finding 4339310383 ( I'm keeping the parameter.
The repair commit message says "matching the base router call site", which is imprecise. The parameter is needed by main's test contract, not by the production caller. I'm not amending the commit only for that wording. Status for maintainers (the bot has taken no CI action):
|
FDecaYed
left a comment
There was a problem hiding this comment.
Approving the sync at 0b27aa77. I re-reviewed the conflict resolutions and the repair commit, and audited dropped changes in both directions. I found no blocking merge errors. Two small merge-introduced issues should ideally be fixed before merge, or in a quick follow-up PR:
-
examples/mimo_bagelstill readsargs.curr_iteration/args.iteration(L351, L583). Main #7932 stopped setting these onargs, and this dev-only example wasn't updated. With PP>1 the data seed is now the same every step, so the numpy/random draws repeat each iteration. Fix: useget_train_state().iteration. -
Main #7494's package-data entry is missing from
pyproject.toml. Dev'spyproject.tomlwas kept, so[tool.setuptools.package-data] "megatron.core.allocator" = ["csrc/*.cpp"]didn't come over. Wheel and sdist installs won't shipvmm_symm_allocator.cpp, so the VMM allocator build fails there. Source-tree and editable CI runs are unaffected.
One combination for the MoE owners to confirm: dev's MOK megakernel (moe_megakernel_backend="mok") and main's moe_virtual_expert_load_balance meet for the first time in this sync, and neither validation rejects the other. The megakernel path returns early (L881), so the virtual-expert wrap/finalize may not pair up. If the combination isn't supported, a mutual-exclusion check would be the safe option.
Agree with keeping precomputed_indices: main #7310's test calls it directly. MBridge tests didn't run because the label is missing.
0b27aa7 to
98b43a0
Compare
|
Reply to review 5609274485 (@FDecaYed). Thanks for the approval.
Both fixes went into the single squashed repair commit. The graph is still 98 imports + 1 merge + 1 repair. To maintainers: this PR still needs the |
|
/ok to test 98b43a0 |
Merge NVIDIA/Megatron-LM main at 0156c21 (the oldest 100 commits of dev..main) into dev at ae76c60. Conflicts were resolved surgically per the nightly-sync skill, keeping dev's dependency triple (pyproject.toml/uv.lock/Dockerfile.ci.dev), CODEOWNERS and dev-only features. tensor_parallel/random.py keeps dev's NVIDIA#8390, the dev port of main NVIDIA#8334 (the only slice commit touching it); post-merge formatting (isort) is folded into this merge commit. Signed-off-by: svcnemo-autobot <svcnemo-autobot@nvidia.com>
Repair main-to-dev sync compatibility for the 2026-10-20 slice (main 0156c21 merged into dev ae76c60): - moe/router: InferenceTopKRouter._compiled_topk_routing accepts and forwards precomputed_indices, matching the base router call site. - tensor_parallel/random: MHCCheckpointManager subclasses CheckpointWithoutOutputManager so main's checkpoint consumers accept it. - moe/token_dispatcher: _HybridEPManager.setup_metadata clears the stale num_permuted_tokens budget when no capacity factor is configured. - tests: align dev-only tests with main API changes (test_training TrainingConfig and TrainState, test_mhc_schedule _model_chunk_state, test_hybrid_hash_routing tensor_parallel import, test_route_metadata_cpu token_dispatcher fake, MTP argmax acceptance-count test). - tests: test_train_step_schedule_plumbing provides the cfg.train, cfg.validation, cfg.ddp and TrainState inputs main's train_step and setup_model_and_optimizer now read. - tests: test_virtual_experts quantile cases select the micro_batch estimation scope that main's quantile estimator implements; the merged default keeps dev's global_batch scope. - tests/test_utils/recipes/gb200/unit-tests.yaml: drop the duplicated tests/unit_tests/ssm/test_gated_delta_net.py bucket produced by the merge, restoring the one-bucket-per-file unit-test scope contract. - tests: test_hybrid_distillation_entry expects the multimodal_dev entry point to leave runtime services and tokenizer vocabulary resolution to initialize_megatron, which main NVIDIA#8272 made their owner. - examples/mimo_bagel/train.py: read the training iteration from get_train_state() for the PP>1 data seed and the loss-alignment audit log, because main NVIDIA#7932 no longer sets args.curr_iteration or args.iteration (review 5609274485). - pyproject.toml: carry main NVIDIA#7494's [tool.setuptools.package-data] entry so wheel and sdist installs ship megatron/core/allocator/csrc/vmm_symm_allocator.cpp (review 5609274485). Signed-off-by: svcnemo-autobot <svcnemo-autobot@nvidia.com>
98b43a0 to
73c827d
Compare
|
/ok to test 73c827d |
Background
This PR merges
mainintodevfor the 100-commit slice850bf4db..0156c213. These are the oldest 100 commits ofdev..main; commit 101 (91d774de) is excluded. The dev base isae76c60e.What changed
b6810054, with parents devae76c60eand main0156c213.73c827d0(fix: post-CI corrections), 14 files, +98/-75:tensor_parallel/random.pyand MoErouter.py/token_dispatcher.pyto API changes from main.random.pykeeps dev's [Dev] refactor(checkpoint): re-architect CheckpointWithoutOutput internals #8390 port of refactor(checkpoint): re-architect CheckpointWithoutOutput internals #8334.examples/mimo_bagel/train.pynow readsget_train_state().iteration, because refactor(training): make TrainState authoritative #7932 no longer setsargs.iteration(review 5609274485).pyproject.tomladds themegatron.core.allocatorcsrc/*.cpppackage-data entry from Fix concurrent VMM allocator extension builds #7494.Python lines: +40287 / -6292 across 410 filesDetails
uv.lockandDockerfile.ci.devare identical to dev.pyproject.tomldiffers from dev only by the Fix concurrent VMM allocator extension builds #7494 entry.git show --remerge-diff b6810054.moe_virtual_expert_load_balancequestion stays deferred to the MoE owners, with no code change.100 source commits (oldest first)
6ae0711 fe3481a f10b91d 3bb482c f9ff958 c4334de ecd9d1b fb65908 7a40475 7a0024e de003e9 1e3c0c6 1c3726f 9460129 c919b5e 86edf3d 0c7d417 07046cc 80781ab 4e23fd6 1e0bb06 6bfec46 f87f092 56a36e5 eb00152 cea0980 51e299e 9bd7ba4 9131593 0101df9 915b1d2 e7c9336 3e95097 a54dd73 9eadd78 c13b6e7 3914483 0adca65 ee6baa2 d7924d3 b597bd1 90dfb4b 1ddc6e4 6e7e6ed 649994e 06b8419 54d99a3 3888b34 438ef00 fb011f2 3e2807c 466cf25 a666bd6 f5186bf adcd90f 502078a 1fc08e5 09462ef c6e38c6 a4b2bee 3ef7c7d 36731ac a4fb6cd 4ab3af1 0900d92 8442db1 e6913b8 a747c1b 31265d3 cca1a74 b1d9750 65ac821 1effc75 4dc7e41 db3a6cd 85c950c 62c063f b607c1b ce852bf d56953f e8a302a 60547a3 66a6d3f e592c04 1bb190d 6c8f14a ebab48f d37b3a8 0e34650 4f86e2c 74c9420 e564873 894b511 d2c430a ebab8e2 474b1e9 7aa615b 13a9d46 fa322ce 0156c21
Tested
megatron-lm@sha256:565e34e2b11f5fd6feeccc75f32253acb86f53d06000a01172a92248c38d8d46(profile megatron-lm v15) on73c827d0:0b27aa77/98b43a0c. Their code and test inputs did not change since then.gpt3_mcore_te_tp1_pp2_dsv4_hybrid_mhc_mtpuses lightweight mode in its dev config, so its pytest golden-value step is skipped by design. Training exited with code 0, and its losses matched between runs.73c827d0: CICD Megatron-LM, Installation Test, Build/validate Megatron-Core, Copyright check and Process group usage check passed.cicd-mbridge-testingwas skipped because theRun MBridge testslabel is not set.