-
Notifications
You must be signed in to change notification settings - Fork 576
Kimi-K3 on layerwise fused export: 3 defect fixes, multimodal support, single-B200 NVFP4 #2218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
fda0fde
96f5459
efac201
937d220
5ee399e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,9 @@ | |
| """Write each decoder layer's quantized checkpoint shard as soon as it is calibrated.""" | ||
|
|
||
| import contextlib | ||
| import contextvars | ||
| import json | ||
| import re | ||
| import warnings | ||
| from pathlib import Path | ||
|
|
||
|
|
@@ -82,8 +84,8 @@ def _module_formats(model: nn.Module) -> set: | |
| } | ||
|
|
||
|
|
||
| def _tied_quantized_modules(model: nn.Module) -> list[str]: | ||
| """Quantized modules sharing a weight with another. | ||
| def _tied_weight_modules(model: nn.Module) -> list[str]: | ||
| """Modules sharing a weight with another, quantized or not. | ||
|
|
||
| Grouped by name, which survives offload: a ``data_ptr`` grouping sees nothing when the | ||
| weights are on meta and would pass vacuously. Falls back to ``data_ptr`` when the model | ||
|
|
@@ -94,7 +96,7 @@ def _tied_quantized_modules(model: nn.Module) -> list[str]: | |
| by_ptr: dict[int, list[str]] = {} | ||
| for name, module in model.named_modules(): | ||
| weight = getattr(module, "weight", None) | ||
| if weight is None or not _is_quantized_module(module): | ||
| if weight is None: | ||
| continue | ||
| key = tied_map.group_key(f"{name}.weight") | ||
| if key is not None: | ||
|
|
@@ -127,16 +129,15 @@ def assert_layerwise_export_supported(model: nn.Module) -> None: | |
| """Raise unless per-layer export is valid for this model.""" | ||
| assert_formats_supported(model, "before calibration") | ||
|
|
||
| tied = _tied_quantized_modules(model) | ||
| tied = _tied_weight_modules(model) | ||
| if tied: | ||
| raise NotImplementedError( | ||
| f"layerwise export does not support weight-tied quantized modules {tied[:6]}: " | ||
| "the whole-model path merges their input_quantizer amaxes via " | ||
| "sync_tied_input_amax so both sides share one input_scale, which a per-layer " | ||
| "pass cannot do because a tie partner may be uncalibrated or already written. " | ||
| "Conversion quantizes every nn.Linear and nn.Embedding, so disabling their " | ||
| "quantizers does not lift this -- tie_word_embeddings models need " | ||
| "export_hf_checkpoint()." | ||
| f"layerwise export does not support weight-tied modules {tied[:6]}: quantized, " | ||
| "the whole-model path merges their input_quantizer amaxes via sync_tied_input_amax " | ||
| "so both sides share one input_scale, which a per-layer pass cannot do because a " | ||
| "tie partner may be uncalibrated or already written; unquantized, save_pretrained " | ||
| "drops the duplicate key and writing shards directly does not. " | ||
| "tie_word_embeddings models need export_hf_checkpoint()." | ||
| ) | ||
|
|
||
| if dist.is_initialized() and dist.size() > 1: | ||
|
|
@@ -146,6 +147,75 @@ def assert_layerwise_export_supported(model: nn.Module) -> None: | |
| ) | ||
|
|
||
|
|
||
| _export_parent: contextvars.ContextVar[nn.Module | None] = contextvars.ContextVar( | ||
| "layerwise_export_parent", default=None | ||
| ) | ||
|
|
||
|
|
||
| @contextlib.contextmanager | ||
| def export_parent(parent: nn.Module): | ||
| """Export the checkpoint for ``parent`` while calibration runs on one of its submodules. | ||
|
|
||
| The decoder layers are the same objects either way, so walking the parent yields | ||
| parent-namespace names, the full config and the untouched towers with no prefixing. | ||
| """ | ||
| token = _export_parent.set(parent) | ||
| try: | ||
| yield | ||
| finally: | ||
| _export_parent.reset(token) | ||
|
|
||
|
|
||
| def _resolve_export_parent(model: nn.Module) -> nn.Module: | ||
| """Return the model the checkpoint should describe. Membership is by identity, not name.""" | ||
| parent = _export_parent.get() | ||
| if parent is None or parent is model: | ||
| return model | ||
| if all(m is not model for m in parent.modules()): | ||
| raise ValueError( | ||
| f"export_parent() was given a {type(parent).__name__} that does not contain the " | ||
| "calibrated model." | ||
| ) | ||
| return parent | ||
|
|
||
|
|
||
| def build_legacy_name_mapper(model: nn.Module): | ||
| r"""Hub-name mapper for transformers < 5, or ``None``. | ||
|
|
||
| ``save_pretrained`` is what reverses ``_checkpoint_conversion_mapping``, and per-layer | ||
| export writes shards directly without it. ``build_reverse_name_mapper`` is no help | ||
| either: it reads transformers 5's ``conversion_mapping`` and raises on 4.x. | ||
|
|
||
| Rules are inverted (the mapping is stored hub -> in-memory) and applied longest-prefix | ||
| first, or a short one shadows a longer (``lm_head`` inside ``model.language_model...``). | ||
| The hub side is a regex: its groups are stripped exactly as save_pretrained strips them | ||
| when reversing the same mapping, and the remainder has its backslashes neutralised so a | ||
| pattern like ``layers\.(\d+)`` substitutes literally instead of raising on ``\d``. | ||
| """ | ||
| mapping = getattr(model, "_checkpoint_conversion_mapping", None) | ||
| if not mapping: | ||
| return None | ||
| rules = sorted( | ||
| ( | ||
| ( | ||
| re.compile("^" + re.escape(mem)), | ||
| re.sub(r"\(.*\)", "", hub.lstrip("^")).replace("\\", "\\\\"), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -euo pipefail
python - <<'PY'
import re
hub = r"layers\.(\d+)"
replacement = re.sub(r"\(.*\)", "", hub.lstrip("^")).replace("\\", "\\\\")
mapped, count = re.compile("^" + re.escape("layers.")).subn(
replacement, "layers.0.weight", count=1
)
assert count == 1
assert mapped == "layers.0.weight", repr(mapped)
PYRepository: NVIDIA/Model-Optimizer Length of output: 270 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- repository conventions and learnings ---'
find /tmp/coderabbit-repo-knowledge/nvidia-model-optimizer-9541ba76 \
-maxdepth 2 -type f -name '*.md' -print \
-exec sh -c 'echo "--- $1"; head -80 "$1"' _ {} \;
printf '%s\n' '--- changed hunk ---'
git diff -- modelopt/torch/export/layerwise_export.py
printf '%s\n' '--- relevant source ---'
cat -n modelopt/torch/export/layerwise_export.py | sed -n '150,235p'
printf '%s\n' '--- mapper definitions and callers ---'
rg -n -C 4 're\.sub|subn|_map|layerwise|legacy|hub' modelopt/torch/export/layerwise_export.py
printf '%s\n' '--- related tests ---'
rg -n -C 3 'layerwise_export|Transformers|layers\\\\\.|map.*name|tied' tests modelopt 2>/dev/null | head -240Repository: NVIDIA/Model-Optimizer Length of output: 50378 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- mapper implementation ---'
cat -n modelopt/torch/export/layerwise_export.py | sed -n '182,225p'
printf '%s\n' '--- mapper references ---'
rg -n -C 5 'build_legacy_name_mapper|legacy_name_mapper|name_mapper|_checkpoint_conversion_mapping|subn\(' \
modelopt/torch/export tests 2>/dev/null | head -220
printf '%s\n' '--- focused tests ---'
find tests -type f -name '*.py' -print0 2>/dev/null |
xargs -0 rg -n -C 4 'build_legacy_name_mapper|layerwise_export|checkpoint_conversion_mapping' 2>/dev/null |
head -220
printf '%s\n' '--- source status ---'
git status --short
git diff --statRepository: NVIDIA/Model-Optimizer Length of output: 27784 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- Transformers compatibility constraints ---'
rg -n -C 3 'transformers([<>=!~]|$)|transformers==|transformers>=' \
pyproject.toml requirements*.txt setup.cfg setup.py modelopt tests 2>/dev/null | head -180
printf '%s\n' '--- repository examples of legacy mapping shapes ---'
rg -n -C 3 '_checkpoint_conversion_mapping|layers\\\\\.|decoder\\\\\.|\\\\\(' \
. --glob '*.py' --glob '*.json' --glob '*.yaml' --glob '*.yml' 2>/dev/null | head -220
printf '%s\n' '--- full relevant export flow ---'
cat -n modelopt/torch/export/layerwise_export.py | sed -n '260,295p;455,488p'Repository: NVIDIA/Model-Optimizer Length of output: 14878 Decode escaped Hub literals before replacement. Line 202 doubles backslashes before 🤖 Prompt for AI Agents |
||
| ) | ||
| for hub, mem in mapping.items() | ||
| ), | ||
| key=lambda r: -len(r[0].pattern), | ||
| ) | ||
|
Comment on lines
+198
to
+207
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CRITICAL Export] The hub side of Evidence that the keys are regexes: the forward direction in this repo applies them as patterns — reverse_key_mapping = {v: k for k, v in self._checkpoint_conversion_mapping.items()}
...
replacement = replacement.lstrip("^")
replacement = re.sub(r"\(.*\)", "", replacement) # <-- missing here
key, n_replace = re.subn(pattern, replacement, key)This helper copies the
Suggested fix — mirror transformers exactly, and pass the replacement as a callable so the template parser never interprets def _hub_replacement(hub: str) -> str:
# Same two steps save_pretrained applies when reversing the mapping.
return re.sub(r"\(.*\)", "", hub.lstrip("^"))
rules = sorted(
((re.compile("^" + re.escape(mem)), _hub_replacement(hub)) for hub, mem in mapping.items()),
key=lambda r: -len(r[0].pattern),
)
def _map(name: str) -> str:
for pattern, replacement in rules:
new, n = pattern.subn(lambda _m, r=replacement: r, name, count=1)
if n:
return new
return nameMinor, same hunk:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in 937d220 |
||
|
|
||
| def _map(name: str) -> str: | ||
| for pattern, replacement in rules: | ||
| new, n = pattern.subn(replacement, name, count=1) | ||
| if n: | ||
| return new | ||
| return name | ||
|
|
||
| return _map | ||
|
|
||
|
|
||
| class LayerwiseExporter: | ||
| """Writes one decoder layer's quantized shard per call, then the tail and index. | ||
|
|
||
|
|
@@ -163,6 +233,7 @@ def __init__( | |
|
|
||
| Runs before calibration, so nothing amax-dependent exists yet. | ||
| """ | ||
| model = _resolve_export_parent(model) | ||
| assert_layerwise_export_supported(model) | ||
| # Splits regroup tensors across the whole state dict; no per-layer pass reverses that. | ||
| _assert_no_split_rules(model) | ||
|
|
@@ -208,10 +279,12 @@ def __init__( | |
| try: | ||
| self._name_mapper = build_reverse_name_mapper(model) | ||
| except Exception as exc: | ||
| warnings.warn( | ||
| f"Reverse name mapper unavailable ({exc}); exported tensor names may not " | ||
| "match the original HF hub checkpoint." | ||
| ) | ||
| self._name_mapper = build_legacy_name_mapper(model) | ||
| if self._name_mapper is None: | ||
| warnings.warn( | ||
| f"Reverse name mapper unavailable ({exc}); exported tensor names may not " | ||
| "match the original HF hub checkpoint." | ||
| ) | ||
|
|
||
| def export_layer( | ||
| self, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| imports: | ||
| base_disable_all: configs/ptq/units/base_disable_all | ||
| default_disabled_quantizers: configs/ptq/units/default_disabled_quantizers | ||
| nvfp4: configs/numerics/nvfp4 | ||
| kv_fp8: configs/ptq/units/kv_fp8 | ||
|
|
||
| metadata: | ||
| recipe_type: ptq | ||
| description: > | ||
| NVFP4 W4A4 on routed experts only, FP8 KV cache, max layerwise calibration, with each | ||
| decoder layer exported to its own shard as soon as it is calibrated. | ||
|
|
||
| Kimi-K3's counterpart to general/ptq/nvfp4_experts_only-kv_fp8_layerwise_export, from | ||
| which it differs only in the narrower expert scope below -- the reason it does not live | ||
| in the general tier. It was validated on a model too large to hold resident, but nothing | ||
| here configures offload: that comes from --offload_folder and the memory budgets. | ||
|
|
||
| Expert scoping is '*.experts.*' rather than '*block_sparse_moe*'. On fine-grained MoE | ||
| models the broader glob also matches shared_experts.*, routed_expert_{up,down}_proj and | ||
| routed_expert_norm -- on Kimi-K3 that is 552 extra modules the vendor deliberately left | ||
| unquantized, one of which is an RMSNorm. '*.experts.*' matches only the routed expert | ||
| projections; shared_experts is missed because its path contains '_experts.' rather than | ||
| '.experts.' (fnmatch semantics, conversion.py). | ||
|
|
||
| An interrupted run resumes without recalibrating or re-exporting finished layers, losing | ||
| at most the in-flight one -- the point of the combination for a run that outlasts its GPU | ||
| session. The resume state lives beside the checkpoint at <export_path>.layerwise_resume | ||
| unless you set layerwise.checkpoint_dir yourself; do not point it at container-local | ||
| storage, or a run that survives its session comes back to a wiped manifest. | ||
|
|
||
| A resumed run never recalibrates the layers it skipped, so the exported checkpoint is | ||
| complete but the in-memory model is not and must not be used for inference. | ||
| quantize: | ||
| algorithm: | ||
| method: max | ||
| layerwise: | ||
| enable: true | ||
| # max only updates _amax, so the exported shard stays valid for its layer. | ||
| calib_mutates_weights: false | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to specify this? Is not this default already? |
||
| # Presence enables per-layer export; the value is replaced with --export_path. | ||
| export_dir: /tmp/modelopt_layerwise_export | ||
| quant_cfg: | ||
| - $import: base_disable_all | ||
| - quantizer_name: '*.experts.*weight_quantizer' | ||
| cfg: | ||
| $import: nvfp4 | ||
| - quantizer_name: '*.experts.*input_quantizer' | ||
| cfg: | ||
| $import: nvfp4 | ||
| - $import: kv_fp8 | ||
| - $import: default_disabled_quantizers | ||
Uh oh!
There was an error while loading. Please reload this page.