Repository navigation
chore: AUT-2472 nightly sync main into dev (2026-10-02) - #7906
Conversation
… failures (NVIDIA#5881) Signed-off-by: Ajay Balasa <abalasa@nvidia.com>
…IA#5570) Signed-off-by: Yongqiang Wang <yongqiang.seagull@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Achyuthan Sivasankar <achyuthan.sivasankar@gmail.com> Signed-off-by: svcnvidia-nemo-ci <svcnvidia-nemo-ci@nvidia.com> Co-authored-by: svcnvidia-nemo-ci <svcnvidia-nemo-ci@nvidia.com>
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
Signed-off-by: Philip Petrakian <ppetrakian@nvidia.com>
Signed-off-by: Teodor-Dumitru Ene <teodord.ene@gmail.com>
…NVIDIA#5864) Signed-off-by: Ali Arda Eker <aeker@nvidia.com>
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com> Signed-off-by: svcnvidia-nemo-ci <svcnvidia-nemo-ci@nvidia.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: svcnvidia-nemo-ci <svcnvidia-nemo-ci@nvidia.com>
Signed-off-by: Charlie Truong <chtruong@nvidia.com>
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
…onsensus (NVIDIA#5331) Signed-off-by: ykarnati <ykarnati@nvidia.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Philip Petrakian <ppetrakian@nvidia.com>
Signed-off-by: William Dykas <wdykas@oci-hsg-cs-001-vscode-03.cm.cluster> Signed-off-by: William Dykas <wdykas@oci-nrt-cs-001-vscode-01.cm.cluster> Co-authored-by: William Dykas <wdykas@oci-hsg-cs-001-vscode-03.cm.cluster> Co-authored-by: William Dykas <wdykas@oci-nrt-cs-001-vscode-01.cm.cluster>
…former_engine_op_fuser` (NVIDIA#5850) Signed-off-by: zhongboz <zhongboz@nvidia.com> Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com>
…ar workflow (NVIDIA#5957) Signed-off-by: Ajay Balasa <abalasa@nvidia.com>
…ry weight and grad norm spikes (NVIDIA#5487) Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com> Signed-off-by: zhongboz <zhongboz@nvidia.com>
…quested (NVIDIA#5918) Signed-off-by: Siddharth Singh <sidsingh@nvidia.com> Signed-off-by: shanmugamr1992 <shanmugamr@nvidia.com> Co-authored-by: Shanmugam Ramasamy <111910568+shanmugamr1992@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…IA#5822) Signed-off-by: Keshav Santhanam <ksanthanam@nvidia.com>
Signed-off-by: Shijie Wang <jaywan@nvidia.com>
Signed-off-by: [Your Name] <your.email@example.com> Signed-off-by: Ajay Balasa <abalasa@nvidia.com> Signed-off-by: svcnemo-autobot <svcnemo-autobot@nvidia.com> Co-authored-by: Ajay Balasa <abalasa@nvidia.com> Co-authored-by: [Co-author Name] <coauthor.email@example.com>
Signed-off-by: Jon Barker <jbarker@aws-dfw-cs-001-login-01.cm.cluster> Co-authored-by: Jon Barker <jbarker@aws-dfw-cs-001-login-01.cm.cluster>
Signed-off-by: wdykas <wdykas@nvidia.com>
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
…IDIA#5912) Signed-off-by: Chen Cui <chcui@nvidia.com>
Signed-off-by: janEbert <janpabloe@nvidia.com>
…VIDIA#6008) Signed-off-by: oliver könig <okoenig@nvidia.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…#6013) Signed-off-by: svcnemo-autobot <svcnemo-autobot@nvidia.com>
svcnvidia-nemo-ci
left a comment
There was a problem hiding this comment.
Follow-up result
F1 is fixed in the inspected source. The generator now captures the workload's exit status immediately, makes extraction non-blocking, accepts the opt-in option, and conditionally publishes the expected artifacts. All four stage argument arrays enable it, matching the existing regression assertions.
The three findings below from review 5447733392 remain supported by the current head. Importing their implementations unchanged from main does not resolve them.
This completes the targeted light follow-up, not a whole-PR review. The previously reviewed tree is unavailable, so the complete inter-review delta could not be reconstructed independently. Inspection covered the prior counterexamples, identifiable subsequent repairs, and their directly affected contracts/tests. No new escalation-eligible regression was established. No tests were executed; acceptance checks and suggested code below require author verification. The required local style guide and determinism-testing integration guide are absent from the protected base tree.
F2 — Make NCCL KV source election agree between sender and receiver
Failure: Give source ranks 0 and 1 identical TP1/PP1 KV shards, and destination rank 2 one complete shard. Call each sender's begin_push_blocks with rank 2's metadata, and call the receiver's begin_pull_blocks with both source metadata records. The sender enumeration plans with only its own source layout, so both senders post sends. The receiver supplies both sources to the planner's smallest-rank election, so it receives only from rank 0. Rank 1 consequently has an unmatched send, and its handle cannot complete normally. Expected: exactly one elected source per replicated shard, with every posted send matched by a receive. This is a backend API counterexample; an integrated production-server path was not established.
Correction: Change _kv_transfers and the push metadata contract so the push side receives the complete source roster or an explicitly agreed elected-source set. Both sides must use the same source election and filter transfers for their own rank. Update metadata producers and direct callers to supply that information. The current push contract carries destination metadata only, so replacing the singleton planner argument alone is insufficient; an exact local-only patch would be misleading.
Preservation: Deduplicate source replicas, not destination replicas: every destination still needs complete KV. Retain nonreplicated TP2→TP1 head merging, explicit PP layer windows, block ordering, and TP/PP metadata normalization. Do not blanket-change the separate conv/recurrent SSM enumeration.
Acceptance — proposed checks: Using the existing test geometry and block IDs [1, 3], mock P2P posting for the replicated-source setup above. Rank 0 must post the required sends, rank 1 none, and receiver operations must match send peers, shapes, counts, and per-peer order. Reversing the source roster must elect rank 0 again. With two destination replicas, both must receive complete data. Retain the real-GPU TP2→TP1 test, specifically torch.equal(buf, expected) and the final all-rank success assertion; add a bounded-time replicated-source transfer check.
F3 — Restore exception-safe MIMO teardown
Failure: In MIMO main, cleanup follows pretrain only on normal return. A setup exception skips distributed shutdown; a training exception after the data provider starts prefetch also skips loader closure and topology destruction. A stubbed pretrain that starts the mocked data provider and raises reproduces the missing cleanup calls. The merge-base implementation guaranteed teardown with nested finally blocks.
Correction: In main, initialize topology = None alongside prefetch_loader = None, put the existing initialization-through-pretrain statements inside try, and replace the trailing cleanup with this suggested block:
finally:
try:
if prefetch_loader is not None:
prefetch_loader.close()
finally:
try:
if topology is not None:
topology.destroy()
finally:
shutdown_distributed()The target is lines 81–148; retain the setup, model-capture hook, nested data provider, and pretrain arguments unchanged apart from indentation. The invariant is that every applicable cleanup is attempted on success or failure, and distributed shutdown is still attempted when an earlier cleanup raises.
Preservation: Keep cleanup order—loader, topology, distributed state—and preserve the loader's bounded, idempotent close. Non-prefetch runs must not attempt loader closure, and failure before topology assignment must not attempt topology destruction.
Acceptance — proposed checks: Mock setup resources and record cleanup calls for successful training, setup failure before topology assignment, and training failure after a mocked loader starts. Assert the applicable calls occur exactly once in the specified order; the original training exception must propagate when cleanup succeeds. Inject loader-close and topology-destroy exceptions separately and assert shutdown still runs. Retain the stuck-worker regression: close warns without raising on join timeout, and producer-stream synchronization remains uncalled.
F4 — Add the required bit-exact extraction-kernel tests
Coverage gap: The two Triton extraction kernels are used by Mamba prefix-cache extraction. Their direct tests perform single reference comparisons, not repeated-execution bit-exact checks. The current determinism suite still lacks the kernel-test directory and manifest required by the pinned light rubric. This is a testing gap, not evidence that either kernel is nondeterministic.
Correction: Add repeated-execution kernel tests under the required determinism-kernel test location and register both extraction kernels in its manifest. Keep kernel implementations unchanged unless those tests expose a failure. The integration guide and registry schema are unavailable, so exact registration code cannot safely be prescribed.
Preservation and acceptance — proposed checks: For each public extraction function, use identical immutable inputs and independently sentinel-initialized outputs, synchronize, and compare output bytes exactly across repeated executions. Cover runtime counts zero, partial, and full; repeated/nonmonotonic SSM indices; and supported state dtypes, including BF16 and FP32. Capture once and replay with changed runtime counts: only active slots may change, while padded slots must retain their sentinel. Preserve the existing conv-clamp assertions for pos=2, d_conv=4, which must read token positions [0, 0, 0, 1], and both existing real-count gating assertions. Also retain the multi-chunk prefix-cache regression: the token-768 snapshot must be registered, and the reuse request must match three Mamba blocks.
Review generated by codex
Reply to review 5454819572 (F2–F4 follow-up)Thanks — F1 is confirmed fixed. I checked F2–F4 again, this time against current
Why not patch these here: this PR imports exactly 100 main commits, adds one merge commit, and allows at most one repair commit, limited to merge and compatibility fixes.
The right fix is a PR on main for F2 and one for F3. If maintainers want, I can prepare them as separate work. If you agree with this disposition, please dismiss this bot review. No code changed for F2–F4, so no tests were run for them. |
FDecaYed
left a comment
There was a problem hiding this comment.
Two potential integration issues I found in the consolidated merge at d92c81ed. This review is limited to merge-introduced issues, not defects already present on either baseline.
1. NCCL-EP zero-copy protection is bypassed by the active overlap schedule
The schedule plan instantiates the GPT-local TransformerLayerNode, whose super call does not forward moe_ncclep_zero_copy. The new protection therefore remains disabled on the active path.
There is a second gap: scheduled release runs before the symmetric-storage check, and its release action can attempt to resize persistent symmetric storage to zero. Main's zero-copy buffers and dev's release/ownership machinery need to be integrated consistently.
Suggested correction: Forward the flag through the active GPT node and treat persistent symmetric buffers as externally owned, excluding them from destructive scheduled release and incompatible alias ownership checks. If that combination cannot be supported safely in this sync, reject zero-copy + scheduled release explicitly. Verify overlap with scheduled release both enabled and disabled, including buffer reuse.
2. GTP-sharded QKV parameters silently lose Muon splitting
The retained QKV tagger reconstructs logical rows using TP only. For example, 8 heads, 4 query groups and head dimension 128 give 2048 logical QKV rows; TP1/GTP2 stores 1024 rows locally, so the tagger sets is_qkv=False and silently bypasses the requested QKV split.
Suggested correction: Keep GTP and manually adapt the core fix from #7117, which is outside this 100-commit slice:
- Derive eligibility and split metadata from the unsharded, unpadded logical layout.
- Gather before splitting/NS, then restore padding and the local GTP shard.
- Preserve dev #7085's per-head/MLA handling and existing mode defaults; carry required metadata through optimizer master copies.
Please avoid replacing the whole Muon implementation or reverting GTP wholesale: #6057 in this slice depends on #4967. A focused compatibility fix should be enough. Acceptance should exercise the actual optimizer factory and compare matched TP/GTP QKV updates, including padding, per-head and fragmented layouts, with consistent collectives across ranks.
These findings are based on source inspection and focused CPU-only logic checks; no GPU/NCCL end-to-end reproduction was performed. The proposed corrections and acceptance checks have not been implemented or run.
There was a problem hiding this comment.
Approving the overall sync at d92c81ed. I don't consider the two integration issues in my earlier review blockers for this PR, but ideally they should be fixed before merging:
- NCCL-EP zero-copy buffer protection in the active overlap/scheduled-release paths.
- GTP-aware Muon QKV tagging and gather-before-split handling.
For the second issue, #7117 is already merged into main, so its fix can also be brought in by a subsequent main-to-dev sync.
Repairs for failures found by local validation of main slice ab20a60 merged into dev f80139c: - tests/unit_tests/inference: give the prefix-caching _StubEngine the cuda_graph_all_prefills attribute the merged DynamicInferenceEngine reads. - megatron/core/optimizer/layer_wise_optimizer.py: tolerate a pg_collection without a tp group (getattr(..., 'tp', None)). - tests/unit_tests/distributed/mfsdp_v2/profiler_utils.py: recover kernel correlation ids without FunctionEvent.linked_correlation_id, which the dev image's torch does not provide. - megatron/core/parallel_state.py: size dynamic data/context-parallel groups against the replicate DP-CP world (with_gtp_remat=False). - gpt3_mcore_te_tp2_pp2_ep4_mhc_overlap_attn_cudagraph{,_nosplit}: refresh dev H100 golden values after main cd4afff (NVIDIA#5916); lm loss and num-zeros match the values main recorded for this change. - megatron/core/inference/contexts/dynamic_context.py: compute Mamba intermediate-state offsets for every prefill chunk, as main does, so the last block boundary of a multi-chunk prompt is cached (required by main dfe9d04 NVIDIA#5952). - tests/unit_tests/test_training.py: give the dev-only training_log fixtures gtp_weight_remat_size=1, the argument main c5ff22b (NVIDIA#4967) made training_log read when computing the global batch size. - megatron/training/training.py: read args.freeze_all_layers with getattr(..., False) in the config-container model-builder path that main ff6b92a (NVIDIA#5926) added, as the same change already does in _forward_backward_grad_context; dev-only tests pass minimal args. - megatron/core/parallel_state.py: with weight rematerialization off (gtp_remat_size == expert_gtp_remat_size == 1), reuse the existing groups of identical membership for the expert GTP weight-remat group and the EGTP-merged expert tp-ep-pp group instead of creating duplicate NCCL process groups. main c5ff22b (NVIDIA#4967) creates them unconditionally; every NCCL group keeps a watchdog thread until process exit, and the extra groups exhausted the thread limit in the transformer unit-test bucket. - tests/functional_tests golden values: drop non-finite iteration-time samples from the 10 golden-value files this sync changes and set start_step/end_step/step_interval to the remaining steps, the layout main uses (cd4afff NVIDIA#5916, 7e6a045 NVIDIA#5997). tools/check_golden_values.py from main bb5647a (NVIDIA#5989) rejects NaN/inf in changed golden values. The H100 files lose only step 1, which lies outside the iteration-time steady window; the two gpt3_7b memory_speed GB200 files keep their even steps 2-24 at step_interval 2. - tests/test_utils/recipes/gb200/gpt-dynamic-inference.yaml: append the read-only GitLab credential to /root/.netrc with `>>` instead of `| tee -a`, so the token is not echoed to stdout and CI logs. The recipe arrives in dev with main 354b5d2 (NVIDIA#6104); this keeps the file otherwise identical to main. - tests/test_utils/python_scripts/generate_jet_trigger_job.py and .gitlab/stages/0{2,3,4}*.yml: restore main's opt-in error extraction (build_test_script, --enable-error-extraction and the four stage flags from main 06bb399, NVIDIA#5831). The earlier dev sync NVIDIA#7055 dropped them without a recorded reason, so this merge kept dev's copies while importing main's test_ci_triage.py, linear_ci.py and 06.triage.yml (e443bd3 NVIDIA#5881, b842f59 NVIDIA#5957), which depend on them. All four files now match main; 01.build.yml keeps dev's base-image pin. - moe/gpt3_mcore_te_tp2_pp1_te_8experts2parallel_{overlap_grad_reduce_param_gather_groupedGEMM,ddp_average_in_collective}_1node and moe/gpt3_moe_mcore_te_tp4_ep2_etp2_pp2_resume_torch_dist_dist_optimizer_1node: refresh the dev GB200 golden values after main cd4afff (NVIDIA#5916). This merge imports the grad-norm/num-zeros fix but kept dev's pre-fix GB200 goldens, so lm loss (exact) and num-zeros failed on dgx_gb200. The values come from the dgx_gb200 golden-value artifacts of CICD run 37688059691 on the previous head 29cf2c4. lm loss, num-zeros and both memory metrics are bit-identical to the goldens the reverted dev sync 5ccdc55 (NVIDIA#7094) regenerated. iteration-time drops its step-1 NaN (start_step 2), as above. - megatron/core/models/gpt/fine_grained_callables.py: forward config.moe_ncclep_zero_copy from the GPT schedule-plan TransformerLayerNode to ScheduleNode, the contract main NVIDIA#5735 added, so free_input never empties NCCL-EP symmetric-memory inputs. megatron/core/transformer/transformer_config.py: reject moe_ncclep_zero_copy with dev's ep_overlap_use_scheduled_tensor_release (NVIDIA#7062), whose scheduled release would own and empty those persistent shared buffers. Addresses review 5470751474 (finding 1). - megatron/core/optimizer/__init__.py and emerging_optimizers.py: with GTP_remat (main NVIDIA#4967), tag fused QKV weights from their TP-local rows before GTP sharding (param._unsharded_shape) instead of the local shard, which silently disabled dev's split-QKV Muon (NVIDIA#7085); rebuild the GTP_remat row shards and strip their alignment padding before the projection/per-head/global QKV split, then restore this rank's shard. Mirrors main's later fix (NVIDIA#7117), which is outside this slice. Addresses review 5470751474 (finding 2). Signed-off-by: svcnemo-autobot <svcnemo-autobot@nvidia.com>
d92c81e to
be3eaaf
Compare
|
/ok to test be3eaaf |
Background
Nightly sync of the oldest exact 100-commit
dev..mainslice intodev, followingmegatron-lm:nightly-sync.f80139c3efc3c0a03c9669df5db5e310f68ad4c7ab20a601247647ab8aab56af1dc0f3adb9e9193a. Commit 101,ec53920e, is excluded.What changed
6cd8c361, with parentsf80139c3(dev) andab20a601(main).be3eaaf9:moe/fused_a2a.py: removes a duplicatedHYBRIDEP_TOKEN_ALIGNMENTconstant. This merge accident brokebuild-docs.generate_jet_trigger_job.pyand.gitlab/stages/02,03and04(main06bb3997a, ci: Enhance nightly/mr/weekly error reporting #5831). The imported triage tooling (ci: integrate nemo-ci-triage with linear issues management for gitlab failures #5881, ci: Integrates the latest config-driven nemo-ci-triage Slack and Linear workflow #5957) needs them.01.build.ymlstays on dev, keeping the 26.04 pin.fine_grained_callables.pynow releases NCCL-EP zero-copy tensors correctly. Test:test_combined_1f1b_tensor_release.py.optimizer/__init__.pyandemerging_optimizers.pysplit QKV for Muon under GTP_remat, with a supportingtransformer_config.pychange. Tests:test_emerging_optimizers.pyandtest_transformer_config.py.lintingare fixed.cd4afffa(Fix gradient-norm undercounting when using EP and TP #5916). They were taken from the CICD 37688059691 artifacts and match dev sync5ccdc5527(chore: AUT-2153 sync exact 100 main commits into dev #7094).gpt-dynamic-inference.yaml: adds the netrc setting.Details
Commit graph relative to dev: 102 commits = 100 imports + 1 merge + 1 repair. This was checked again on
be3eaaf9.Imported commits, oldest first (
git rev-list --reverse f80139c3..ab20a601)e443bd3 bd0872d cfb116f 5a88c57 337c061 7012adb 12c05a2 368fa88 58bf14e ddaa315 a046281 d207685 cc5b092 96d4159 890247c e4ea859 b842f59 8d16c67 602fad0 bb5647a 6cd6ea5 52cf20d 411a5d8 c5398de 51e915a 15d4b34 acd0e35 4464d1c 4f72cc0 ff60360 81770cb a69bbb0 0a7f372 4f65717 6f0db6f 72411e5 7ee2580 650b783 2d754e6 af3d240 648bc01 066145e dfe9d04 50cc27a 21fe0fe 10c9383 7e6a045 c5ff22b cd4afff 7ec1bce 543b803 076f61f 876016a fc57daa 10cd30c 4b18b26 07e958f c4b4d59 866efa4 6f7bcd4 bacd340 8a424b8 4da9212 7557c02 053fa07 541d5ee c922805 2576c15 818b258 2658704 157c023 bc67abd 8a63dd5 0a447f5 f43ff6b b78cfd5 3ff70c0 54045a5 efb3963 c7d1865 e598899 e1b2a2f 542ff53 6183f9d eab6551 3f88509 8f1c99c 77c8772 9436656 8e57bb6 354b5d2 ab16414 ff6b92a 3fd8f33 175b4cc ce8a8a7 9d416ee 0081c84 dc3c9ab ab20a60
77c87725Add disaggregated KV transfer backends #5861,a69bbb09Add encoder prefetch for heterogeneous MIMO training #5833), so they should be fixed on main. F4 (648bc011Inference: Optimized triton kernels to extract mamba states in prefix caching #5866) is already fixed on main byf6c33bde4(test(determinism): add kernel replay tests and a PR coverage gate #7148), which comes after this slice.be3eaaf9.multi-approval-bot.ymlstays deleted.be3eaaf9, up from 2880 ond92c81ed. The 5 new lines are dev-only optimizer lines that F6 rewrites on purpose. There are no CODEOWNERS or dependency-pin warnings.f11f0340is clean.Tested
Local H100 runs, image
sha256:4a507fcf41bd3b89cb540bed1c1d80e4df4fed0b31e058290b073a5783eb3434.On
be3eaaf9, for the unit groups F5/F6 touch: 5/5 passed.**/*.py: 2153 passedpipeline_parallel: 203 passeda2a_overlap: 46 passedmodels: 659 passedtransformer: 2250 passedOther rows were not rerun because F5/F6 does not affect them; their earlier passes are kept.
Earlier, on the merge repair:
common/moe_perffails the same way on both merge parents (a pre-existingRandomSTE.generatorAttributeError).Not tested locally:
be3eaaf9.GitHub CI:
d92c81ed: CICD 37758767237 had 158 jobs pass and 3 skipped. The only failure iscicd-mbridge-testing(the Megatron-Bridge App installation lookup failed).common/moe_perfhits the same AttributeError there; CI marks it optional.be3eaaf9: pending.