Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 32 additions & 3 deletions tensorrt_llm/_torch/models/modeling_minimaxm3_vl.py
Original file line number Diff line number Diff line change
Expand Up @@ -962,6 +962,31 @@ def from_dict_or_obj(cls, source: Any) -> "CLIPVisionConfig":
return cls(**filtered)


# ---------------------------------------------------------------------------
# Layer norm.
# ---------------------------------------------------------------------------


class MiniMaxVLLayerNorm(nn.LayerNorm):
"""``nn.LayerNorm`` whose parameter init is skipped on meta tensors.

``reset_parameters`` fills weight/bias via ``aten.fill_.Scalar``, which
``MetaInitMode`` rejects, so a plain ``nn.LayerNorm`` anywhere in the
``__init__`` tree aborts meta-init for the whole model and drops the loader
onto its regular-init fallback (hundreds of GB of host allocation per rank
for M3). Every layer-norm slot here is covered by the checkpoint, so the
skipped values are always overwritten at load time.
"""

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring asserts "every layer-norm slot here is covered by the checkpoint." That's the safety argument for the whole change, and it's checked by nothing — if a future config adds a norm the weight mapper doesn't populate, the parameter stays uninitialized on the meta path and silently produces garbage rather than failing loudly. Is there an existing post-load check that no parameter is left on meta? If so, worth naming it here; if not, that's the guard this comment is standing in for.


def reset_parameters(self) -> None:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The meta gate is the right call — worth noting modeling_nemotron.py:59 has the unconditional variant of this (reset_parameterspass), which leaves torch.empty storage on any checkpoint-free build. Since this is now the second copy of the same workaround, consider hoisting it to a shared module (e.g. next to the other norms in _torch/modules/) so the next VLM doesn't rediscover it; happy for that to be a follow-up rather than this PR.

# Skip only on meta: off meta the ones/zeros are still needed, otherwise
# a module built without a checkpoint keeps uninitialized storage.
# ``weight`` is None when elementwise_affine=False.
if self.weight is not None and self.weight.is_meta:
return
super().reset_parameters()


# ---------------------------------------------------------------------------
# Patch embedding (Conv3d).
# ---------------------------------------------------------------------------
Expand Down Expand Up @@ -1192,9 +1217,13 @@ def __init__(self, config: CLIPVisionConfig, dtype: torch.dtype):
super().__init__()
self.embed_dim = config.hidden_size
self.self_attn = MiniMaxVLEncoderSelfAttention(config, dtype)
self.layer_norm1 = nn.LayerNorm(self.embed_dim, eps=config.layer_norm_eps, dtype=dtype)
self.layer_norm1 = MiniMaxVLLayerNorm(
self.embed_dim, eps=config.layer_norm_eps, dtype=dtype
)
self.mlp = MiniMaxVLEncoderMLP(config, dtype)
self.layer_norm2 = nn.LayerNorm(self.embed_dim, eps=config.layer_norm_eps, dtype=dtype)
self.layer_norm2 = MiniMaxVLLayerNorm(
self.embed_dim, eps=config.layer_norm_eps, dtype=dtype
)

def forward(
self,
Expand Down Expand Up @@ -1265,7 +1294,7 @@ def __init__(self, config: CLIPVisionConfig, dtype: torch.dtype):

self.embeddings = MiniMaxVLPatchEmbedding(config, dtype)
# NOTE: the typo "layrnorm" matches the published checkpoint key.
self.pre_layrnorm = nn.LayerNorm(embed_dim, eps=config.layer_norm_eps, dtype=dtype)
self.pre_layrnorm = MiniMaxVLLayerNorm(embed_dim, eps=config.layer_norm_eps, dtype=dtype)
self.encoder = MiniMaxVLEncoder(config, dtype)

if config.position_embedding_type != "rope" or config.rope_mode != "3d":
Expand Down
2 changes: 0 additions & 2 deletions tests/integration/test_lists/waives.txt
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,7 @@ full:B300/accuracy/test_llm_api_pytorch.py::TestDeepSeekV3Lite::test_nvfp4_4gpus
full:B300/accuracy/test_llm_api_pytorch.py::TestDeepSeekV4FlashBase::test_fp8_4gpus_static_eplb[moe_backend=WIDEEP] SKIP (https://nvbugs/6546609)
full:B300/accuracy/test_llm_api_pytorch.py::TestGemma3_1BInstruct::test_fp8_prequantized[torch_compile=True] SKIP (https://nvbugs/6475346)
full:B300/accuracy/test_llm_api_pytorch.py::TestLagunaXS::test_fp8 SKIP (https://nvbugs/6525011)
full:B300/accuracy/test_llm_api_pytorch.py::TestMiniMaxM3::test_auto_dtype[tp_size=8-ep_size=8] SKIP (https://nvbugs/6445375)
full:B300/accuracy/test_llm_api_pytorch.py::TestMiniMaxM3::test_mxfp8[use_msa=False] SKIP (https://nvbugs/6424188)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_nvfp4[use_msa=False] is being unwaived here for B300, but the same parametrization is still waived on B200 (line 160), GB200 (line 210) and GB300 (line 243) under different bugs, and B300 keeps test_mxfp8[use_msa=False] waived under 6424188 (this line). If B300 is the only platform where this case is expected to pass, that's fine — but please confirm it actually ran green rather than inferring it from the test_auto_dtype fix. Otherwise this re-enables a case that fails everywhere else and the next failure gets triaged as a new regression.

full:B300/accuracy/test_llm_api_pytorch.py::TestMiniMaxM3::test_nvfp4[use_msa=False] SKIP (https://nvbugs/6445375)
full:B300/accuracy/test_llm_api_pytorch.py::TestMistralLarge3_675B::test_nvfp4_4gpus[latency_moe_trtllm] SKIP (https://nvbugs/6529874)
full:B300/accuracy/test_llm_api_pytorch.py::TestNemotronV3Ultra::test_nvfp4_8gpus[attention_dp_off-trtllm] SKIP (https://nvbugs/6474894)
full:B300/accuracy/test_llm_api_pytorch.py::TestStep3_7::test_fp8_block_scales[tp_size=4-ep_size=4-mtp_nextn=3] SKIP (https://nvbugs/6539941)
Expand Down
63 changes: 63 additions & 0 deletions tests/unittest/_torch/models/test_minimax_m3_vl.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import numpy as np
import pytest
import torch
import torch.nn as nn
from PIL import Image
from safetensors import safe_open
from transformers import AutoConfig, AutoProcessor, AutoTokenizer
Expand All @@ -31,6 +32,7 @@
from tensorrt_llm._torch.models.modeling_minimaxm3 import get_text_config
from tensorrt_llm._torch.models.modeling_minimaxm3_vl import (
CLIPVisionConfig,
MiniMaxVLLayerNorm,
MiniMaxVLPatchEmbedding,
MiniMaxVLPatchMerger,
MiniMaxVLVisionModel,
Expand All @@ -47,6 +49,7 @@
reanchor_multimodal_checkpoint_keys,
split_multimodal_weights,
)
from tensorrt_llm._torch.models.modeling_utils import MetaInitException, MetaInitMode

# ---------------------------------------------------------------------------
# Shared helpers (mirror the conventions used by test_minimax_m3.py).
Expand Down Expand Up @@ -812,6 +815,66 @@ def test_patch_merger_rejects_unaligned_input():
merge(x)


# ---------------------------------------------------------------------------
# meta-init compatibility (CPU, no checkpoint).
# ---------------------------------------------------------------------------


def _build_tiny_vision_model() -> MiniMaxVLVisionModel:
cfg = CLIPVisionConfig.from_dict_or_obj(_tiny_vision_config())
return MiniMaxVLVisionModel(
config=cfg,
text_hidden_size=16,
projector_hidden_size=16,
dtype=torch.float32,
)


def test_vision_tower_builds_under_meta_init():
"""The vision tower must construct inside ``MetaInitMode``.

See :class:`MiniMaxVLLayerNorm` for why a plain ``nn.LayerNorm`` here aborts
meta-init for the whole M3 model.
"""
with MetaInitMode():
model = _build_tiny_vision_model()

layer_norms = [m for m in model.modules() if isinstance(m, nn.LayerNorm)]
assert layer_norms, "expected layer norms in the vision tower"
# pre_layrnorm + layer_norm1/2 per encoder layer.
assert len(layer_norms) == 1 + 2 * len(model.vision_model.encoder.layers)
for ln in layer_norms:
assert ln.weight.is_meta
assert ln.bias.is_meta


def test_meta_init_still_rejects_plain_layer_norm_init(monkeypatch):
"""Control for :func:`test_vision_tower_builds_under_meta_init`.

Restoring the upstream ``reset_parameters`` must bring the exception back,
otherwise that test could pass without the skip doing any work.
"""
monkeypatch.setattr(MiniMaxVLLayerNorm, "reset_parameters", nn.LayerNorm.reset_parameters)
with pytest.raises(MetaInitException, match="fill_"):
with MetaInitMode():
_build_tiny_vision_model()


def test_layer_norm_off_meta_init_matches_upstream():
"""Off meta the skip must not engage, else a checkpoint-free build keeps
uninitialized ``torch.empty`` storage instead of ones/zeros."""
ln = MiniMaxVLLayerNorm(8, dtype=torch.float32)
assert torch.equal(ln.weight, torch.ones(8))
assert torch.equal(ln.bias, torch.zeros(8))


def test_layer_norm_without_affine_params_builds_under_meta_init():
"""``elementwise_affine=False`` registers ``weight`` as ``None``."""
with MetaInitMode():
ln = MiniMaxVLLayerNorm(8, elementwise_affine=False, dtype=torch.float32)
assert ln.weight is None
Comment on lines +823 to +875

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

for test_name in \
  test_vision_tower_builds_under_meta_init \
  test_meta_init_still_rejects_plain_layer_norm_init \
  test_layer_norm_off_meta_init_matches_upstream \
  test_layer_norm_without_affine_params_builds_under_meta_init
do
  echo "== $test_name =="
  rg -n -F "$test_name" tests/integration/test_lists/test-db tests/integration/test_lists/qa || true
done

Repository: NVIDIA/TensorRT-LLM

Length of output: 381


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== test file status =="
git status --short -- tests/unittest/_torch/models/test_minimax_m3_vl.py

echo "== matching test-list entries by file or suite =="
rg -n -i -F "test_minimax_m3_vl" tests/integration/test_lists/test-db tests/integration/test_lists/qa || true
rg -n -i -F "minimax" tests/integration/test_lists/test-db tests/integration/test_lists/qa || true

echo "== available list files =="
find tests/integration/test_lists -maxdepth 2 -type f -print | sort | sed -n '1,160p'

echo "== nearby test definitions =="
rg -n "^(def test_|class Test)" tests/unittest/_torch/models/test_minimax_m3_vl.py | sed -n '1,120p'

Repository: NVIDIA/TensorRT-LLM

Length of output: 16415


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== test-list scope =="
sed -n '1,180p' tests/integration/test_lists/test-db/README.md
sed -n '1,180p' tests/integration/test_lists/qa/README.md

echo "== unit-test entries in integration lists =="
rg -n -i "unittest/|tests/unittest|pytest.*unittest" tests/integration/test_lists/test-db tests/integration/test_lists/qa | sed -n '1,160p'

echo "== file history summary =="
git log -5 --oneline -- tests/unittest/_torch/models/test_minimax_m3_vl.py

Repository: NVIDIA/TensorRT-LLM

Length of output: 29614


Register the new unit tests in CI.

  • Added tests: test_vision_tower_builds_under_meta_init, test_meta_init_still_rejects_plain_layer_norm_init, test_layer_norm_off_meta_init_matches_upstream, and test_layer_norm_without_affine_params_builds_under_meta_init.
  • The test file and these test functions are absent from the CI test lists. Add the four tests to a suitable entry, such as tests/integration/test_lists/test-db/l0_cpu.yml.
  • Run pytest tests/unittest/ for this change.
  • Coverage verdict: insufficient.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unittest/_torch/models/test_minimax_m3_vl.py` around lines 823 - 875,
Register the four MiniMax M3 VL tests—test_vision_tower_builds_under_meta_init,
test_meta_init_still_rejects_plain_layer_norm_init,
test_layer_norm_off_meta_init_matches_upstream, and
test_layer_norm_without_affine_params_builds_under_meta_init—in an appropriate
CI test-list entry such as l0_cpu.yml, then run pytest tests/unittest/ to verify
discovery and execution.

Sources: Coding guidelines, Path instructions



# ---------------------------------------------------------------------------
# full multimodal smoke (CUDA + real checkpoint).
# ---------------------------------------------------------------------------
Expand Down
Loading