Repository navigation
chore: AUT-2547 nightly sync main into dev (2026-10-16) - #8305
svcnemo-autobot wants to merge 102 commits into
Conversation
…A#6251) Signed-off-by: guihong-nv <guihongl@nvidia.com>
Signed-off-by: Ajay Balasa <abalasa@nvidia.com>
…IDIA#5908) Signed-off-by: ilml <tolong@nvidia.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: wdykas <wdykas@nvidia.com>
…IDIA#6646) Signed-off-by: Ajay Balasa <abalasa@nvidia.com>
Signed-off-by: dimapihtar <dpykhtar@nvidia.com> Signed-off-by: Dmytro Pykhtar <dpykhtar@nvidia.com>
Signed-off-by: Charlie Truong <chtruong@nvidia.com>
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
Signed-off-by: svcnemo-autobot <svcnemo-autobot@nvidia.com>
Signed-off-by: Keshav Santhanam <ksanthanam@nvidia.com>
Signed-off-by: ykarnati <ykarnati@nvidia.com>
Signed-off-by: Ritesh Patel <ripatel@nvidia.com> Co-authored-by: gautham-kollu <gkollu@nvidia.com>
Signed-off-by: Jiangfei Duan <jiangfeid@nvidia.com>
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
Signed-off-by: Kwangjun Ahn <kwangjuna@nvidia.com>
Signed-off-by: dimapihtar <dpykhtar@nvidia.com> Signed-off-by: Dmytro Pykhtar <dpykhtar@nvidia.com>
Signed-off-by: Philip Petrakian <ppetrakian@nvidia.com>
Signed-off-by: rprenger <rprenger@nvidia.com> Signed-off-by: Cory Ye <cye@nvidia.com> Signed-off-by: Ryan Prenger <rprenger@nvidia.com> Co-authored-by: Cory Ye <cye@nvidia.com>
Signed-off-by: svcnemo-autobot <svcnemo-autobot@nvidia.com>
Signed-off-by: ykarnati <ykarnati@nvidia.com>
…ry point (NVIDIA#6684) Signed-off-by: ilml <tolong@nvidia.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: wdykas <wdykas@nvidia.com>
Signed-off-by: Anil Thomas <anthomas@nvidia.com> Co-authored-by: OpenAI Codex <codex@openai.com>
Signed-off-by: YangFei1990 <feiw@nvidia.com>
Signed-off-by: Laura Dang <laurad@nvidia.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Jiangfei Duan <jiangfeid@nvidia.com>
…IDIA#6717) Signed-off-by: Deepak Narayanan <dnarayanan@nvidia.com>
Signed-off-by: Shiqing Fan <shiqingf@nvidia.com> Signed-off-by: Deepak Narayanan <dnarayanan@nvidia.com> Co-authored-by: Shiqing Fan <shiqingf@nvidia.com>
Signed-off-by: Charlie Truong <chtruong@nvidia.com>
Signed-off-by: Charlie Truong <chtruong@nvidia.com>
|
/ok to test d743194 |
|
/review mode=strict |
svcnvidia-nemo-ci
left a comment
svcnvidia-nemo-ci
left a comment
There was a problem hiding this comment.
Strict review — changes required; review incomplete
Findings: CRITICAL: 7, IMPORTANT: 1, SUGGESTION: 0. Overall risk is high. The most impactful finding is an SSRF bypass in remote image fetching. The inspected multimodal paths also contain deterministic construction failures and an image-feature indexing error.
The remaining inspection task cannot be completed within this bounded run and its final continuation. The changeset contains 589 files and a 2.7-MB diff; substantial source, tests, and configuration changes remain uninspected. The required full-file, identifier-use, and cross-component checks are consequently unfinished. The task remains pending rather than being falsely marked complete. This is not clearance of the remaining changes, and no coverage-policy override is claimed. All currently validated findings are included below; review_progress records the outstanding work.
No tests were executed. Acceptance checks below are proposed; suggested code requires author verification.
Suggested structural corrections
Findings 2–3: construct the optional hybrid MoE spec conditionally
In get_hybrid_layer_spec_te, replace the configuration extraction/assertion with:
moe_layer = IdentityOp
if config is not None and config.num_moe_experts is not None:
moe_layer = ModuleSpec(
module=MoETransformerLayer,
submodules=TransformerLayerSubmodules(
pre_mlp_layernorm=TENorm,
mlp=get_moe_module_spec(
use_te=True,
num_experts=config.num_moe_experts,
moe_grouped_gemm=config.moe_grouped_gemm,
),
mlp_bda=get_bias_dropout_add,
),
)Replace the current eager moe_layer=ModuleSpec(...) argument with moe_layer=moe_layer. Leave the Mamba, attention, and dense-MLP specs unchanged. Dense patterns never select the MoE slot; expert patterns receive the configured builder. This also removes the unsupported helper keyword.
Finding 4: validate individual layers within a block spec
In LLaVAModel.init, import TransformerBlockSubmodules from megatron.core.transformer.transformer_block. Replace the assertions inside the non-hybrid branch with:
layer_specs = (
language_transformer_layer_spec.layer_specs
if isinstance(language_transformer_layer_spec, TransformerBlockSubmodules)
else [language_transformer_layer_spec]
)
for layer_spec in layer_specs:
assert isinstance(layer_spec.submodules, TransformerLayerSubmodules)
attention = layer_spec.submodules.self_attention
assert isinstance(attention.submodules, SelfAttentionSubmodules)
assert (
attention.submodules.core_attention == TEDotProductAttention and HAVE_TE
), "Sequence/Context Parallelism is supported only with TE DotProductAttention."Pass the original block spec unchanged to GPTModel. This retains the per-layer check and validates every local layer rather than bypassing the TE restriction for blocks.
Finding 5: preserve pipeline coordinates when creating the block spec
Add these arguments to the new get_gpt_decoder_block_spec call:
vp_stage=vp_stage,
pp_rank=pg_collection.pp.rank() if pg_collection is not None else None,The absent-collection case retains the helper's existing fallback; supplied groups determine the PP rank.
Finding 7: count the features the encoder actually emits
In the dynamic-resolution loop of expand_image_tokens, replace the count calculation with the contract already used by LLaVA's dynamic preprocessing:
num_embeddings = (h // patch_dim) * (w // patch_dim)
if not module._drop_vision_class_token:
num_embeddings += module._class_token_len
if do_pixel_shuffle:
num_embeddings //= 4
if module._conv_merging:
num_embeddings //= 4
per_image_embeddings.append(num_embeddings)This uses the resolved class-token count, including constructor/configuration overrides. Native merger configuration must also be wired consistently as described in finding 6. Do not unconditionally add CLS tokens or divide every encoder's count by four. Retained-CLS plus pixel shuffle is not a newly supported combination: the current pixel-shuffle implementation expects patch-only inputs.
Finding 8: recognize only the deleted built-in metadata identities
Before the custom-class import in _get_tokenizer_model_class, add narrow compatibility handling:
legacy_text_models = {
"default_tokenizer": "DefaultTokenizerText",
"gpt_tokenizer": "GPTTokenizer",
"bert_tokenizer": "BertTokenizer",
"mamba_tokenizer": "MambaTokenizer",
"t5_tokenizer": "T5Tokenizer",
}
legacy_text_identities = {
(f"megatron.core.tokenizers.text.models.{module}", name)
for module, name in legacy_text_models.items()
}
identity = (metadata.get("class_path"), metadata.get("class_name"))
if identity in legacy_text_identities:
from megatron.core.tokenizers.text import MegatronTokenizerText
return MegatronTokenizerText
if identity == (
"megatron.core.tokenizers.vision.models.default_tokenizer",
"DefaultTokenizerVision",
):
from megatron.core.tokenizers.vision import MegatronTokenizerVision
return MegatronTokenizerVisionKeep the current import behavior for other explicit class identities and the generic fallback for metadata without a class. The inspected deleted built-ins were thin wrappers around these generic classes.
Note: formal approval is not available for this request, so this review is posted as a comment only.
Review generated by codex
| ): | ||
| raise ValueError(f"Refusing to fetch image from non-public address: {parsed.hostname}") | ||
| req = urllib.request.Request(url, headers={"User-Agent": _IMAGE_FETCH_USER_AGENT}) | ||
| with _no_redirect_opener.open(req, timeout=_IMAGE_FETCH_TIMEOUT_S) as response: |
There was a problem hiding this comment.
[CRITICAL Correctness] Pin the validated destination through the actual HTTP connection
Failure: The public-address check resolves the hostname with gethostbyname, but urllib subsequently resolves the original URL again. In a proxy-free server, an attacker-controlled hostname can return a public A record for validation and 127.0.0.1 for connection establishment. Posting an image_url such as http://attacker.example:8080/private therefore makes the frontend issue a GET to a loopback service. The chat endpoint invokes this fetch before submitting inference; later image-decoding failure does not undo that request. Mixed A/AAAA answers also escape a check examining only one IPv4 result. Expected: the address actually contacted must satisfy the public-address policy.
Correction: Change _extract_image_url_bytes and its HTTP transport so resolution, validation, and connection use the same approved address. Validate every candidate the transport may select; prevent unchecked re-resolution and proxy routing. Preserve the original hostname for HTTP Host and HTTPS SNI/certificate verification. The exact transport implementation needs verification; rewriting an HTTPS URL to an IP alone is not a complete fix.
Preservation / proposed acceptance: With a mocked resolver returning public first and loopback on a subsequent lookup, assert that no connection or HTTP request reaches loopback. Repeat with a public A/private AAAA combination. Retain rejection of direct private/link-local destinations and 301/302/303/307/308 redirects, the five-second timeout, and the 20-MiB response cap. Public HTTP/HTTPS image URLs and base64 data URLs should still work; HTTPS must still verify the requested hostname. These are static attack-path reasoning and proposed tests, not executed results.
| attn_mask_type = AttnMaskType.padding_causal | ||
|
|
||
| if config is not None: | ||
| assert config.num_moe_experts is not None, ( |
There was a problem hiding this comment.
[CRITICAL Correctness] Keep dense hybrid configs out of the MoE-only branch
Failure: The provider now passes language_config for every Nemotron5 hybrid. A normal dense nemotron5-hybrid-8b configuration has num_moe_experts=None, so this assertion aborts model construction. Previously the helper constructed the Mamba/attention/dense-MLP stack without requiring experts. Passing config=None is not a workaround: the new MoE expression is evaluated eagerly too.
Correction: In get_hybrid_layer_spec_te, construct the MoE spec only when a non-null expert count is supplied; otherwise leave its slot at IdentityOp, the dataclass default. Do not merely delete the assertion and call the MoE helper with None: its backend contract asserts a non-null expert count. The review body supplies a conditional replacement that also uses the current helper signature.
Preservation / proposed acceptance: Build the spec with config=None and with a dense config, for both padding=False and True; both must succeed without invoking the MoE helper. Retain Mamba/attention/dense-MLP specs and causal versus padding-causal masks. Construct a small dense M*- hybrid and verify its layer types and state-dict keys remain unchanged. With eight experts, the MoE slot must instead contain the configured builder and an E layer must construct rather than silently become an identity.
| use_te=True, | ||
| num_experts=num_experts, | ||
| moe_grouped_gemm=moe_grouped_gemm, | ||
| moe_use_legacy_grouped_gemm=False, |
There was a problem hiding this comment.
[CRITICAL Correctness] Call the dev MoE helper with its supported signature
Failure: With a valid MoE config, for example num_moe_experts=8, spec construction reaches this call and raises TypeError: get_moe_module_spec() got an unexpected keyword argument 'moe_use_legacy_grouped_gemm'. The inspected helper accepts only use_te, num_experts, and moe_grouped_gemm. Consequently the new Nemotron6-MoE provider path cannot construct its spec.
Correction: Remove this unsupported keyword from this call, not from unrelated APIs. Keep the returned MlpBuilder in TransformerLayerSubmodules.mlp; that is the current contract. The conditional patch in the review body combines this change with the separate dense-config repair.
Preservation / proposed acceptance: With TE available, call the helper using an eight-expert configuration with grouped GEMM both enabled and disabled. Each call must succeed and select the corresponding experts implementation, then construct a small hybrid containing an E layer with eight configured experts. Dense/None configurations must continue to avoid this MoE-only call, and attention padding-mask selection must remain unchanged. These checks are proposed, not run.
| config=language_config, padding=padding | ||
| ) | ||
| elif getattr(args, 'num_experts', None): | ||
| language_transformer_layer_spec = get_gpt_decoder_block_spec( |
There was a problem hiding this comment.
[CRITICAL Correctness] Adapt LLaVA's SP/CP validation to the new block-spec return type
Failure: Selecting num_experts now returns a TransformerBlockSubmodules object. With sequence parallelism or CP greater than one, LLaVAModel immediately reads .submodules from that object, raising AttributeError. The block dataclass has layer_specs and layer_norm, not submodules. GPTModel itself supports this block representation, so rejecting it before construction defeats the new branch.
Correction: Update the non-hybrid SP/CP validator in LLaVAModel.__init__ to inspect each local layer spec for a block input and the single layer for the existing ModuleSpec input. Keep the TE-attention restriction; do not simply skip validation for blocks. Pass the original spec unchanged to GPTModel. The review body contains the normalization/checking patch.
Preservation / proposed acceptance: Under initialized groups and TE, construct a small non-hybrid MoE LLaVA using the returned block spec at TP=2/SP=true and separately CP=2. Construction must not raise this AttributeError. A block whose second layer uses non-TE core attention must still fail the supported-backend check. Retain the per-layer ModuleSpec cases in test_process_embedding_token_parallel, including its padded THD and unpadded SBHD output-shape assertions.
| use_transformer_engine=use_te, | ||
| normalization=args.normalization, | ||
| qk_l2_norm=getattr(args, 'qk_l2_norm', False), | ||
| ) |
There was a problem hiding this comment.
[CRITICAL Correctness] Forward the virtual pipeline stage when slicing MoE specs
Failure: For a non-hybrid MoE model with PP=2 and VPP=2, model_provider(vp_stage=0, ...) calls the new block builder without that stage. The builder slices local layers using get_transformer_layer_offset, which asserts that vp_stage is provided under VPP. This fails before LLaVA construction, even with SP/CP disabled. The training caller supplies the stage explicitly.
Correction: Add vp_stage=vp_stage and pp_rank=pg_collection.pp.rank() if pg_collection is not None else None to this call, as shown in the review body. The block must be sliced using the same pipeline coordinates as the language model, while absent explicit groups retain the existing fallback.
Preservation / proposed acceptance: Use a four-layer config with PP=2/VPP=2 and a mixed dense/MoE pattern. Build each physical/virtual stage and assert that each gets its intended one-layer spec, collectively covering all four layers exactly once without the assertion. Use a supplied PP collection to ensure selection follows that collection rather than a different global rank. Retain PP=1 and PP=2/no-VPP layer counts and vp_stage=None behavior when VPP is disabled. This is independent of the block-type validation failure above.
| from nvlm.internvit import get_internvit300M_layer_spec | ||
|
|
||
| vision_transformer_layer_spec = get_internvit300M_layer_spec(use_te=use_te) | ||
| elif vision_model_type in ("pixtral-vit", "pixtral-vit-large", "qwen-vl", "kimi-vit"): |
There was a problem hiding this comment.
[CRITICAL Correctness] Complete construction support for the newly selected native encoders
Failure: The added Pixtral/Qwen/Kimi branches are unreachable through this provider. With dynamic resolution and vision_model_type='qwen-vl', the earlier call to get_vision_model_config raises ValueError: unknown vision model type qwen-vl. Static resolution fails even earlier in get_num_image_embeddings. Even a direct LLaVAModel caller supplying its own native vision config eventually hits the unconditional geometry-helper call, whose implementation rejects all four new names.
Correction: Add native-only architecture resolution to get_vision_model_config, using the corresponding encoder registry entries, and extend the geometry contract used by both provider and LLaVA constructor. Native encoders have no CLS tokens; plain Pixtral emits one token per patch, whereas Pixtral-Large, Qwen, and Kimi emit one per 2×2 patch group. Wire the merger flag consistently for dynamic accounting. Fixing only the config rejection leaves the next constructor failure intact. A complete integration patch requires verification, so no speculative registry-wide rewrite is suggested.
Preservation / proposed acceptance: Parameterize provider construction over the four new names and assert expected architecture fields plus successful lightweight LLaVA construction. For a 4×4 patch grid, assert 16 tokens for plain Pixtral and four for each merged encoder, matching actual output. Preserve CLIP's retained/dropped CLS behavior, SigLIP's no-CLS behavior, existing RADIO configuration/overrides, and existing pixel-shuffle accounting; do not replace those established defaults wholesale with registry defaults.
| per_image_embeddings = [] | ||
| for row in imgs_sizes_cpu: | ||
| h, w = row[0], row[1] | ||
| num_embeddings = (h // patch_dim) * (w // patch_dim) |
There was a problem hiding this comment.
[CRITICAL Implementation] Include retained class tokens in dynamic image expansion
Failure: For dynamic RADIO with patch size 16, eight retained CLS tokens, no pixel shuffle, and two images sized 32×32 and 32×64, this code allocates 4+8 image positions. The encoder actually emits 12+16 features: RADIO prepends CLS tokens per image, and LLaVA removes them only when requested. The second image's mask consequently starts at feature four instead of twelve, so it consumes features from the first image. The later maximum-index guard does not detect under-counting. Expected: expansion and per-image offsets must match the post-encoder/post-shuffle feature stream.
Correction: In expand_image_tokens, add the resolved module._class_token_len only when _drop_vision_class_token is false, then apply configured pixel-shuffle and native-merger reductions. The review body gives the replacement consistent with LLaVA's dynamic count contract. Do not blanket-add CLS tokens for no-CLS encoders or blanket-divide unmerged encoders by four.
Preservation / proposed acceptance: For the two-image example, assert counts [12,16], a second-image offset of 12, and that distinguishable per-image feature sentinels land only in their own positions. With CLS dropped, expect [4,8]. For dropped-CLS RADIO with 4×4 and 4×8 patch grids plus pixel shuffle, expect [4,8]; no-CLS native 2×2 mergers should also use [4,8] when configured, while unmerged Pixtral uses [16,32]. Retain the static num_tiles × img_seq_len path. Tests are proposed, not executed.
| if metadata.get('tokenizer_class', None): | ||
| return getattr(metadata['tokenizer_class_path'], metadata['tokenizer_class_name']) | ||
| if metadata.get('class_name', None): | ||
| module = importlib.import_module(metadata['class_path']) |
There was a problem hiding this comment.
[IMPORTANT Compatibility] Resolve metadata referring to the deleted built-in tokenizer classes
Failure: Previously valid metadata such as {'library':'null-text','model_type':'gpt','class_name':'GPTTokenizer','class_path':'megatron.core.tokenizers.text.models.gpt_tokenizer'} loaded successfully with vocab_size=16. The previous metadata writer could emit that class identity. This resolver now imports that identity, but the module was deleted, so loading raises ModuleNotFoundError. The same applies to the other deleted built-in text wrappers and DefaultTokenizerVision. Expected: existing tokenizer metadata remains loadable after this simplification.
Correction: Before importing explicit classes, map only the six deleted built-in identities to MegatronTokenizerText or MegatronTokenizerVision, or restore equivalent compatibility modules. The review body contains a narrow resolver patch. Do not ignore all class metadata: that would break the newly supported custom-class path.
Preservation / proposed acceptance: Load the literal null-text metadata above without tokenizer files and assert tokenize('1 2') == [1,2] and eod == 15. Parameterize the five old text identities, and exercise the old vision identity with the null-multimodal backend. Metadata without explicit class fields and newly written generic metadata must still load. Retain test_own_metadata_class's assertion that the result is an instance of CustomTokenizerClass. These are proposed checks, not run results.
|
Sync author note on review 5556506430 (head I traced all 8 findings to source. Each flagged path is semantically identical to
The same defects therefore exist on |
FDecaYed
left a comment
FDecaYed
left a comment
There was a problem hiding this comment.
One merge-specific issue to address before merging: with CP>1, hybrid KDA, linear_cp_layout=contiguous, and the default cp_partition_mode=zigzag, the new batch path supplies contiguous shards, but the retained chunkwise path converts them as zigzag again, scrambling token order.
For this sync, I suggest rejecting contiguous hybrid CP for the affected GDN/KDA layers until the layout contracts are integrated. Existing dev paths and the new Mamba support can stay unchanged.
Confirmed with a CPU-only source-prefix probe; no full GPU numerical test.
d743194 to
c5f9446
Compare
|
@FDecaYed thanks, you're right. When CP>1 and The fix is in the repair commit, now
|
|
/ok to test c5f9446 |
FDecaYed
left a comment
FDecaYed
left a comment
There was a problem hiding this comment.
Thanks, the CP guard looks good, and all four new cases passed in CI. My original CP concern is resolved.
One small CI fix remains: the MTP test failure comes from the merge preview including dev #8105. The new cross-entropy branch reads configuration fields missing from _make_forward_stub() in test_multi_token_prediction.py.
Please add these defaults to the stub config and rerun the MTP tests:
cross_entropy_loss_fusion=False,
cross_entropy_fusion_impl="native",This is separate from the CP guard; no production-code fallback is needed.
Single repair commit for the 2026-10-16 main->dev sync of the 100-commit main slice ending at 302ed23 onto dev dee456a. Dependencies - pyproject.toml: add gigatoken to the training and mlm extras, which code imported from main requires. - uv.lock: regenerated with uv 0.7.2 (Dockerfile pin); adds gigatoken 0.10.0, awkward 2.14.0 and awkward-cpp 57; no removals. Tests adapted from main's API to dev's API - transformer/test_multi_token_prediction.py: dev's MTPLossLoggingHelper exposes reduced values via reduce_loss_in_tracker()/tracker["values"]. - data/test_get_batch.py: dev's pretrain_hybrid.get_batch reads args.linear_cp_layout and args.context_parallel_size. - inference/contexts/test_dynamic_prefix_caching.py: on dev the engine records the Mamba match count before admission; set it like the engine. - models/mimo/test_mimo_1f1b_schedule.py: pass use_layer_wise_param_layout so DistributedDataParallelConfig agrees with the layer-wise optimizer. - transformer/experimental_attention_variant/test_attention_variant_dsa.py: dynamic context parallel on dev requires max_seqlen_per_dp_cp_rank. - transformer/test_multi_token_prediction.py (main NVIDIA#6119 speculative-decoding capture test): dev stores the MTP input on model._decoder_hidden_states_cache, not on inference_context.mtp_decoder_hidden_states. Golden values - moe/gpt3_moe_mcore_te_ep8_resume_torch_dist_dist_muon dev H100 golden refreshed from the exact merge-candidate run: main 0560fbb (NVIDIA#6192) makes the LayerWise Muon child grad statistics span the intra distributed-optimizer group; reverting only that line reproduces dev's previous golden values exactly. - tests: set max_seqlen_per_dp_cp_rank in dev dynamic-CP/scheduler test configs (transformer_layer, transformer_config_dynamic_cp, dsv4_hybrid_attention_cp) to satisfy main NVIDIA#5902 validation; dev arguments.py already derives it when a sequence-packing scheduler is set. - tests: transformer/test_thd_cuda_graph.py balanced DSA dynamic-pack case with max_seqlen_per_dp_cp_rank=None now expects main NVIDIA#5902's earlier ModelParallelConfig validation message. - tests: extend main NVIDIA#6284 mimo overlap-lifecycle optimizer mock with the dev _optimizer_state_offloader/_layer_wise_non_distopt_child attributes (NVIDIA#6244). - tests: extend main NVIDIA#6119 HybridModel MTP forward stub with dev moe_n_hash_layers and pg_collection.dp_cp used by the dev forward. Megatron-FSDP v2 symmetric memory (requester-approved alignment with main) - experimental/dbuffer.py, experimental/parameter_group.py: take main's version. Symmetric-memory MFSDP raises "requires PyTorch 2.12 or later" on PyTorch builds without torch.distributed._symmetric_memory .is_symm_mem_tensor, as on main. This drops dev's allocation-flag fallback (ef8d090, 4a534df); with it, the symmetric path's losses diverged across ranks on dev's PyTorch 2.12 pre-release image and test_fully_shard_symmetric_memory_matches_default_and_profiles_nccl[3] failed on the merge candidate. - mfsdp_v2 tests: skip the symmetric-memory cases on such builds (test_symmetric_memory.py, the symmetric_memory params of test_overlap.py and test_cuda_graph.py, and test_mcore_adapter.py nccl_ub). They run again once the image provides is_symm_mem_tensor. Hybrid context parallelism (review 5570704991) - models/hybrid/hybrid_block.py: HybridStack rejects linear_cp_layout='contiguous' with context parallelism when the segment contains GDN ('G') or KDA ('K') layers. main NVIDIA#6776 feeds contiguous shards to linear layers, but dev's GDN/KDA chunkwise and headwise paths still read their input layout from cp_partition_mode (zigzag by default) and would scramble token order. Contiguous Mamba/GDP and the zigzag GDN/KDA paths are unchanged. - ssm/test_hybrid_block.py: cover the GDN/KDA rejection and the zigzag GDN and contiguous Mamba builds under CP=2. - tests: transformer/test_multi_token_prediction.py TestMultiTokenPredictionHybrid (main NVIDIA#6119) stub config sets cross_entropy_loss_fusion=False and cross_entropy_fusion_impl="native" (the TransformerConfig defaults); dev NVIDIA#8105 HybridModel.forward reads them when labels are given (review 5573599695). Signed-off-by: svcnemo-autobot <svcnemo-autobot@nvidia.com>
c5f9446 to
898ccb7
Compare
|
/ok to test 898ccb7 |
|
@FDecaYed thanks. Both defaults are now in I reran |
|
/ok to test 898ccb7 |
Background
Nightly main→dev sync (2026-10-16): the oldest exact 100-commit prefix of
dev..main, tip302ed2304135a75c295a86e84be9b526c81f803c, merged into dev basedee456ab738d400d113590592079e54ae68654fd. Commit 101 (efb6ed57) is excluded. Python: +28124 / -2883 lines, 294 files.What changed
dee456ab: 100 unchanged main commits, 1 merge6ca045f0(parentsdee456ab,302ed230), 1 signed repair commit898ccb7d.gigatokenadded to training/mlm extras;uv.lockregenerated (uv 0.7.2; adds gigatoken 0.10.0, awkward 2.14.0, awkward-cpp 57). Dockerfile.ci.dev unchanged.max_seqlen_per_dp_cp_rank.moe/gpt3_moe_mcore_te_ep8_resume_torch_dist_dist_muon(main Scope layerwise optimizer collectives to be per module for mulitmodal mimo training #6192).dbuffer.py/parameter_group.pytaken (requester-approved); symmetric-memory tests skip on dev's PyTorch.HybridStackraisesValueErrorfor CP>1 +linear_cp_layout="contiguous"with GDN/KDA layers (they followcp_partition_mode, zigzag by default; main CP layout integration #6776). New CP=2 test, 4 cases.cross_entropy_loss_fusion=Falseandcross_entropy_fusion_impl="native", read by dev [dev] misc fixes while bring-up chunk-wise cuda graph #8105'sHybridModel.forward.Details
megatron/training/training.pyfrom main (sync-guide override). No dev-deleted files restored.c5f94462, same asd743194c): 2218 flagged lines; 2088 merge-base lines changed by main; 98 dev-only + 32 repair lines each dispositioned (main commit cited, reformat, or requester-approved). 0 merge accidents, 0 restorations.git show --remerge-diff 6ca045f0(186 files).Tested
agent-sandbox@sha256:12fb4544fa55dfa61f3038cbcfe7338ed1f241f40f5c8f7333bd60ffc91d3e83(earlier2cbc1243passes carried by build-input equivalence).ssmonc5f94462: 321 passed, 8 skipped, 0 failed (incl. CP=2 cases). MTP: PR CI onc5f94462failed only this bucket (run 39255571541); local 8×H100 rerun withcross_entropy_loss_fusion=False: 188 passed, 0 failed; thecross_entropy_fusion_implline is never read with fusion off (all 9 reads checked). PR CI decidesssm/gated_delta_net/test_gdn_parallel.py.common/moe_perfandmoe/gpt_dynamic_inference_tp4_pp1_ep4_16B_inference_optimized_swiglu_logitsmatch(dev base fails them locally too).mimo_hetero_gtp_checkpoint_round_trip(HAVE_GTP=False); mfsdp_v2 symmetric-memory.d743194c186 passed, 8 skipped;898ccb7dpending.100 imported main commits (ancestry order)