Repository navigation
[None][fix] Tighten GLM-5.3-Flash modeling_v2 routing and fused MTP step #22088
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| """The check that a GLM-5.3-Flash target module's ``UNCERTIFIED_GENERIC_CALLS`` names the stock code it runs. | ||
|
|
||
| A target's generic path runs stock tensorrt_llm modules that no catalog entry certifies. The module that imports | ||
| them lists them in ``UNCERTIFIED_GENERIC_CALLS``, so a dependency on stock code is visible and the list shrinks as | ||
| entries replace stock calls. The check reads the source and imports nothing. Each target's test directory runs it on | ||
| that target's declaring modules. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import ast | ||
| from pathlib import Path | ||
|
|
||
| PACKAGE = "tensorrt_llm._torch._experimental.modeling_v2" | ||
| FAMILY = "glm5_next" | ||
| FAMILY_DIR = ( | ||
| Path(__file__).resolve().parents[6] / "tensorrt_llm/_torch/_experimental/modeling_v2/models" | ||
| ) / FAMILY | ||
|
|
||
| #: tensorrt_llm imports that compute nothing, so the generic path need not declare them: types and configs it | ||
| #: reads, enums it passes, the metadata it is handed and its logger. | ||
| _NON_COMPUTE_IMPORTS = frozenset( | ||
| { | ||
| "AllReduceStrategy", | ||
| "AttentionMetadata", | ||
| "AuxStreamType", | ||
| "Mapping", | ||
| "ModelConfig", | ||
| "QuantAlgo", | ||
| "QuantConfig", | ||
| "logger", | ||
| } | ||
| ) | ||
|
|
||
|
|
||
| def is_entry_module(module: str, target: str) -> bool: | ||
| """Whether ``module`` is modeling_v2 code the declaring module may call: catalog entries, the native loader, the | ||
| family's ``ops`` and the target's own ``ops``.""" | ||
| family_package = f"{PACKAGE}.models.{FAMILY}" | ||
| roots = ( | ||
| f"{PACKAGE}.catalog", | ||
| f"{PACKAGE}._native", | ||
| f"{family_package}.ops", | ||
| f"{family_package}.{target}.ops", | ||
| ) | ||
| return any(module == root or module.startswith(root + ".") for root in roots) | ||
|
|
||
|
|
||
| def declared(tree: ast.Module) -> tuple[str, ...] | None: | ||
| """The string tuple assigned to ``UNCERTIFIED_GENERIC_CALLS``, or None if the module has none.""" | ||
| for node in tree.body: | ||
| if isinstance(node, ast.Assign) and any( | ||
| isinstance(t, ast.Name) and t.id == "UNCERTIFIED_GENERIC_CALLS" for t in node.targets | ||
| ): | ||
| return tuple(ast.literal_eval(node.value)) | ||
| return None | ||
|
|
||
|
|
||
| def stock_imports(tree: ast.Module, target: str) -> set[str]: | ||
| """Every tensorrt_llm name the module imports with ``from ... import`` outside modeling_v2 entries, module level, | ||
| function local or for type checking only, except the names that compute nothing.""" | ||
| imported = set() | ||
| for node in ast.walk(tree): | ||
| if ( | ||
| isinstance(node, ast.ImportFrom) | ||
| and node.level == 0 | ||
| and node.module is not None | ||
| and node.module.split(".")[0] == "tensorrt_llm" | ||
| and not is_entry_module(node.module, target) | ||
| ): | ||
| imported.update( | ||
| f"{node.module}.{alias.name}" | ||
| for alias in node.names | ||
| if alias.name not in _NON_COMPUTE_IMPORTS | ||
| ) | ||
| return imported | ||
|
|
||
|
|
||
| def declaring_modules(target: str) -> list[Path]: | ||
| """The modules of ``target`` that declare ``UNCERTIFIED_GENERIC_CALLS``.""" | ||
| return sorted( | ||
| p | ||
| for p in (FAMILY_DIR / target).rglob("*.py") | ||
| if declared(ast.parse(p.read_text())) is not None | ||
|
Comment on lines
+85
to
+86
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. Audit generic modules that lack a declaration. A new target helper can import stock code without defining As per path instructions, flag “tests that can silently pass.”
|
||
| ) | ||
|
|
||
|
|
||
| def check_names_every_stock_import(path: Path, target: str) -> None: | ||
| """The list of the module at ``path`` in ``target`` names every stock import, and nothing it does not import, so | ||
| it cannot go stale.""" | ||
| tree = ast.parse(path.read_text()) | ||
| names, imported = set(declared(tree)), stock_imports(tree, target) | ||
| assert imported <= names, ( | ||
| f"imported but not in UNCERTIFIED_GENERIC_CALLS: {sorted(imported - names)}" | ||
| ) | ||
| assert names <= imported, ( | ||
| f"UNCERTIFIED_GENERIC_CALLS names what it does not import: {sorted(names - imported)}" | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| """The split target serves text only: its multimodal admission hook refuses a request that carries image or video | ||
| input, which the forward would otherwise serve as its text alone.""" | ||
|
|
||
| import types | ||
|
|
||
| import pytest | ||
|
|
||
| from tensorrt_llm._torch._experimental.modeling_v2.models.glm5_next.glm_5_3_flash_nvfp4__sm_100__default.modeling import ( # noqa: E501 | ||
| ModelingV2Glm53FlashNvfp4Sm100Default as Target, | ||
| ) | ||
| from tensorrt_llm._torch.models.modeling_multimodal_mixin import MultimodalModelMixin | ||
|
|
||
| pytestmark = pytest.mark.cpu_only | ||
|
|
||
| _HOOK = "validate_multimodal_request_data" | ||
|
|
||
|
|
||
| def test_the_target_defines_the_admission_hook(): | ||
| """``PyExecutor._validate_request`` calls the hook by the name ``MultimodalModelMixin`` declares.""" | ||
| assert callable(getattr(MultimodalModelMixin, _HOOK, None)) | ||
| assert _HOOK in vars(Target) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "mm_data", | ||
| [ | ||
| {"image": {"pixel_values": None, "image_grid_thw": None}}, | ||
| {"video": {"pixel_values_videos": None, "video_grid_thw": None}}, | ||
| ], | ||
| ids=["image", "video"], | ||
| ) | ||
| def test_multimodal_input_is_refused_at_admission(mm_data): | ||
| # The hook reads no model state, so it runs without building the model. | ||
| with pytest.raises(ValueError, match="serves text only"): | ||
| getattr(Target, _HOOK)(types.SimpleNamespace(), mm_data) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| """Each module of the split target that declares ``UNCERTIFIED_GENERIC_CALLS`` (its ``modeling.py`` must) names there | ||
| every stock tensorrt_llm name it imports with ``from ... import``, and nothing else. | ||
|
|
||
| The check is ``_glm5_next_generic_calls``; it reads the source and imports nothing.""" | ||
|
|
||
| import pytest | ||
|
|
||
| __extra_import_path__ = [".."] | ||
| from _glm5_next_generic_calls import ( # noqa: E402 | ||
| FAMILY_DIR, | ||
| check_names_every_stock_import, | ||
| declaring_modules, | ||
| ) | ||
|
|
||
| pytestmark = pytest.mark.cpu_only | ||
|
|
||
| TARGET = "glm_5_3_flash_nvfp4__sm_100__default" | ||
| MODULES = declaring_modules(TARGET) | ||
|
|
||
|
|
||
| def test_modeling_declares_the_generic_calls(): | ||
| assert FAMILY_DIR / TARGET / "modeling.py" in MODULES, ( | ||
| f"{TARGET}/modeling.py does not declare UNCERTIFIED_GENERIC_CALLS" | ||
| ) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("path", MODULES, ids=lambda p: str(p.relative_to(FAMILY_DIR / TARGET))) | ||
| def test_uncertified_generic_calls_name_every_stock_import(path): | ||
| check_names_every_stock_import(path, TARGET) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Detect ordinary stock imports.
A target can add
import tensorrt_llm._torch.modules.gated_mlp as mlpand usemlp.GatedMLPwithout changing the declaration. This check ignores the import, so the audit still passes. Detect these imports and their used names, or reject this import form. Add a source-fixture test for the chosen behavior.As per path instructions, flag “tests that can silently pass.”
Source: Path instructions