Skip to content

Commit 67cdc97

Browse files
Address review: distilled-export layout, hub-id self-check, NVFP4 scale merge
export_distilled_megatron_to_hf.py chose its MoE layout with `use_moe_grouped_gemm`, but it now rejects quantized checkpoints, so every checkpoint it handles was written by distill.py's unquantized branch using grouped GEMM. For an architecture with no `experts.linear_fc1` rule the two disagreed, and the script would have built SequentialMLP over a grouped-GEMM checkpoint. It mirrors distill.py's choice now. `_verify_exported_keys` bailed out whenever the source was not a local directory, which is the documented invocation -- every README and PR snippet passes a hub repo id. The guard meant to protect user runs on architectures CI never sees was therefore inactive for exactly those runs. It now fetches just `*.safetensors.index.json`, and still skips when the source cannot be reached. The NVFP4 merge kept each expert's block scales but replaced `weight_scale_2` with the cross-expert maximum, inflating every quieter expert's effective scale by `scale_2_max / scale_2_i`. Values round-tripped, which is why the reference check passed, but the FP4 range was wasted. Block scales are now rescaled onto the merged global scale; measured on the 256-expert half-depth Qwen3.5, worst-case relative error against the reference drops from 0.16667 to 0.15331. Both packed paths share one helper so they cannot drift. Grouped-expert packing only forwarded the four suffixes it names, silently dropping anything else (`bias` on a future `add_bias_linear` MoE); leftovers now assert. `load_modelopt_megatron_checkpoint` reads the checkpoint keys already, so it now checks the stored expert layout against the built model instead of trusting the config. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
1 parent 215d4aa commit 67cdc97

3 files changed

Lines changed: 58 additions & 17 deletions

File tree

examples/megatron_bridge/export_distilled_megatron_to_hf.py

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@
6969
is_vlm_config,
7070
load_mbridge_model_from_hf,
7171
load_modelopt_megatron_checkpoint,
72-
use_moe_grouped_gemm,
7372
)
7473

7574
# Megatron-Bridge checkpoint iteration directories use names like ``iter_0000100``.
@@ -256,11 +255,8 @@ def main(args: argparse.Namespace):
256255
_bridge, _provider, _model, full_model, _tokenizer = load_mbridge_model_from_hf(
257256
hf_model_name_or_path=args.student_hf_path,
258257
trust_remote_code=args.trust_remote_code,
259-
moe_grouped_gemm=use_moe_grouped_gemm(
260-
args.student_hf_path,
261-
trust_remote_code=args.trust_remote_code,
262-
force_sequential=args.no_moe_grouped_gemm,
263-
),
258+
# Mirrors distill.py's unquantized branch
259+
moe_grouped_gemm=not args.no_moe_grouped_gemm,
264260
provider_overrides={
265261
"tensor_model_parallel_size": args.tp_size,
266262
"pipeline_model_parallel_size": args.pp_size,

modelopt/torch/export/unified_export_megatron.py

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929

3030
import torch
3131
import torch.distributed
32-
from huggingface_hub import hf_hub_download
32+
from huggingface_hub import hf_hub_download, snapshot_download
3333
from huggingface_hub.errors import EntryNotFoundError
3434
from safetensors import safe_open
3535
from safetensors.torch import save_file
@@ -50,6 +50,7 @@
5050
QUANTIZATION_W4A16_NVFP4,
5151
)
5252
from .plugins.hf_checkpoint_utils import (
53+
_is_hf_hub_offline,
5354
copy_hf_ckpt_remote_code,
5455
copy_non_safetensor_files_from_ckpt,
5556
load_multimodal_components,
@@ -445,16 +446,25 @@ def save_pretrained(
445446

446447
def _verify_exported_keys(self, save_directory, pretrained_model_name_or_path) -> None:
447448
"""Raise if the export dropped tensors the source has: a missing rule emits nothing."""
448-
if pretrained_model_name_or_path is None or not os.path.isdir(
449-
str(pretrained_model_name_or_path)
450-
):
451-
return # hub id: not worth a download inside export
449+
if pretrained_model_name_or_path is None:
450+
return
451+
source_dir = str(pretrained_model_name_or_path)
452+
if not os.path.isdir(source_dir):
453+
# A repo id is the documented invocation, so fetch just the index rather than skip.
454+
try:
455+
source_dir = snapshot_download(
456+
repo_id=source_dir,
457+
allow_patterns=["*.safetensors.index.json"],
458+
local_files_only=_is_hf_hub_offline(),
459+
)
460+
except Exception:
461+
return # source unreachable: skip rather than fail an otherwise good export
452462
index_file = Path(save_directory) / "model.safetensors.index.json"
453463
if not index_file.exists():
454464
return
455465
with open(index_file) as f:
456466
exported = set(json.load(f)["weight_map"])
457-
source = _read_checkpoint_keys(pretrained_model_name_or_path)
467+
source = _read_checkpoint_keys(source_dir)
458468
if not source:
459469
return
460470

@@ -1267,6 +1277,11 @@ def collect(suffix):
12671277
weights = collect(".weight")
12681278
if not weights:
12691279
return
1280+
handled = (".weight", ".weight_scale", ".weight_scale_2", ".input_scale", ".output_scale")
1281+
unhandled = {k.split(".", 1)[1] for k in per_expert if not k.endswith(handled)}
1282+
assert not unhandled, (
1283+
f"{prefix}: grouped-expert packing has no rule for {sorted(unhandled)}"
1284+
)
12701285
# Record against the packed prefix, as _pack_name_remapping does for the other packed path.
12711286
if qformat in (None, QUANTIZATION_NONE):
12721287
self._record_excluded_module(prefix)
@@ -1282,9 +1297,8 @@ def collect(suffix):
12821297
self._state_dict[prefix] = merged_weight
12831298
else:
12841299
if scales_2:
1285-
# NVFP4 keeps each expert's block scales; only the global scale is merged.
1286-
merged_scale = torch.stack(scales, dim=0)
1287-
merged_scale_2 = torch.max(torch.stack(scales_2, dim=0), dim=0)[0]
1300+
# NVFP4 keeps each expert's block scales, rescaled onto the merged global scale.
1301+
merged_scale, merged_scale_2 = self._merge_nvfp4_expert_scales(scales, scales_2)
12881302
else:
12891303
merged_scale, merged_scale_2 = torch.max(torch.stack(scales, dim=0), dim=0)[0], None
12901304
self._state_dict[prefix] = to_quantized_weight(
@@ -1771,6 +1785,20 @@ def _self_attention_scaling(
17711785
# FP8 KV Cache is supported in VLLM; NVFP4 supported in TRTLLM
17721786
self.kv_cache_dtype = kv_cache_dtype
17731787

1788+
@staticmethod
1789+
def _merge_nvfp4_expert_scales(scales: list, scales_2: list):
1790+
"""Merge per-expert NVFP4 scales onto one global scale, preserving each expert's FP4 range.
1791+
1792+
Each expert's block scales were derived against its own ``scale_2``; rescaling them by
1793+
``scale_2_i / scale_2_max`` keeps the quieter experts from losing a mantissa bit.
1794+
"""
1795+
merged_scale_2 = torch.max(torch.stack(scales_2, dim=0), dim=0)[0]
1796+
stacked_2 = torch.stack(scales_2, dim=0).reshape(-1, *([1] * scales[0].dim()))
1797+
merged_scale = (
1798+
torch.stack(scales, dim=0).to(torch.float32) * (stacked_2 / merged_scale_2)
1799+
).to(scales[0].dtype)
1800+
return merged_scale, merged_scale_2
1801+
17741802
def _pack_name_remapping(self, module, prefix, layer_type=None, is_mtp=False, transpose=True):
17751803
"""Pack per-expert weights into one tensor; ``transpose`` for HF [E, in, out] layouts."""
17761804
if is_mtp:
@@ -1812,8 +1840,9 @@ def _pack_name_remapping(self, module, prefix, layer_type=None, is_mtp=False, tr
18121840
merged_weight_scale = None
18131841
else:
18141842
# NVFP4
1815-
merged_weight_scale_2 = torch.max(torch.stack(weight_scale_2_list, dim=0), dim=0)[0]
1816-
merged_weight_scale = torch.stack(weight_scale_list, dim=0)
1843+
merged_weight_scale, merged_weight_scale_2 = self._merge_nvfp4_expert_scales(
1844+
weight_scale_list, weight_scale_2_list
1845+
)
18171846
if transpose:
18181847
merged_weight_scale = merged_weight_scale.transpose(-2, -1).contiguous()
18191848

modelopt/torch/utils/plugins/mbridge.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,22 @@ def load_modelopt_megatron_checkpoint(
230230
print_rank_0("Language-model-only checkpoint: loading into the VLM's `.language_model`.")
231231
model = [get_language_model(m)[0] for m in unwrapped_model]
232232

233+
# The layout is baked into the checkpoint, so a model built with the other one silently
234+
# mismatches. The keys are already read above, so check rather than trust the config.
235+
ckpt_grouped = any(".experts.weight0" in key for key in checkpoint_keys)
236+
ckpt_sequential = any(".local_experts." in key for key in checkpoint_keys)
237+
if ckpt_grouped or ckpt_sequential:
238+
built_sequential = any(
239+
".local_experts." in name for m in unwrap_model(model) for name, _ in m.named_modules()
240+
)
241+
if ckpt_sequential != built_sequential:
242+
raise ValueError(
243+
f"{megatron_path} stores MoE experts as "
244+
f"{'SequentialMLP' if ckpt_sequential else 'grouped GEMM (TEGroupedMLP)'} but the "
245+
f"model was built as {'SequentialMLP' if built_sequential else 'grouped GEMM'}. "
246+
"Pass the same --no_moe_grouped_gemm setting used to write the checkpoint."
247+
)
248+
233249
# Restore the ModelOpt state before loading weights.
234250
# has_modelopt_state / load_modelopt_state resolves the latest iter_* directory
235251
if restore_modelopt_state:

0 commit comments

Comments
 (0)