From 7118da39143ee1b68063a9a89bbf855b7e7ad29f Mon Sep 17 00:00:00 2001 From: ayaangazali Date: Thu, 20 Aug 2026 22:08:27 -0700 Subject: [PATCH 1/4] fix(multimodal): stop the media loaders from importing torch `from dynamo.common.multimodal import ImageLoader` pulled in `torch` and the rest of the ML stack. `image_loader.py` depends only on asyncio, PIL and the local http helpers, but the package `__init__` re-exported `AsyncEncoderCache` and the embedding-transfer members eagerly, and those reach `torch` through `dynamo.common.memory.multimodal_embedding_cache_manager`. Because the eager imports run before the loader re-export, importing the lightweight name paid for the heavy one. The effect is that the URL-loading and request-deduplication logic cannot be imported or unit-tested in a CPU-only environment without installing torch or stubbing it into `sys.modules` first. Measured which submodules are actually heavy rather than deferring everything: `async_encoder_cache` and `embedding_transfer` import torch; `audio_loader`, `image_loader` and `video_loader` do not. Only the heavy names move behind a module-level `__getattr__` (PEP 562), which caches each resolved attribute into the module globals so repeat access costs nothing. The three loaders stay eager. `EMBEDDING_SENDER_FACTORIES` and `EMBEDDING_RECEIVER_FACTORIES` had to become lazy too. They are built from the embedding-transfer classes, so evaluating them at import time would have kept the eager import no matter what the re-exports did. Their contents, including the NIXL_READ `max_items=0` lambda and its existing FIXME, are unchanged. The public surface is unchanged: `__all__` is the same, `__dir__` reports it, and the names are re-declared under `TYPE_CHECKING` so type checkers still resolve them. Fixes #11172. Signed-off-by: ayaangazali --- .../src/dynamo/common/multimodal/__init__.py | 136 +++++++++++++----- .../tests/multimodal/test_lazy_imports.py | 44 ++++++ 2 files changed, 148 insertions(+), 32 deletions(-) create mode 100644 components/src/dynamo/common/tests/multimodal/test_lazy_imports.py diff --git a/components/src/dynamo/common/multimodal/__init__.py b/components/src/dynamo/common/multimodal/__init__.py index f99aa7c249d2..b774d5ed68c5 100644 --- a/components/src/dynamo/common/multimodal/__init__.py +++ b/components/src/dynamo/common/multimodal/__init__.py @@ -1,44 +1,116 @@ # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 -"""Multimodal utilities for Dynamo components.""" +"""Multimodal utilities for Dynamo components. -from collections.abc import Callable +`AsyncEncoderCache` and the embedding-transfer members import `torch`, while the +image, audio and video loaders do not. Re-exporting all of them eagerly meant +`from dynamo.common.multimodal import ImageLoader` pulled in the whole ML stack, +so the loaders could not be imported or unit-tested without it. The heavy names +resolve on first access instead (PEP 562); the loaders stay eager because they +cost nothing. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any -from dynamo.common.constants import EmbeddingTransferMode -from dynamo.common.multimodal.async_encoder_cache import AsyncEncoderCache from dynamo.common.multimodal.audio_loader import AudioLoader -from dynamo.common.multimodal.embedding_transfer import ( - AbstractEmbeddingReceiver, - AbstractEmbeddingSender, - LocalEmbeddingReceiver, - LocalEmbeddingSender, - NixlReadEmbeddingReceiver, - NixlReadEmbeddingSender, - NixlWriteEmbeddingReceiver, - NixlWriteEmbeddingSender, - TransferRequest, -) from dynamo.common.multimodal.image_loader import ImageLoader from dynamo.common.multimodal.video_loader import VideoLoader -EMBEDDING_SENDER_FACTORIES: dict[ - EmbeddingTransferMode, Callable[[], AbstractEmbeddingSender] -] = { - EmbeddingTransferMode.LOCAL: LocalEmbeddingSender, - EmbeddingTransferMode.NIXL_WRITE: NixlWriteEmbeddingSender, - EmbeddingTransferMode.NIXL_READ: NixlReadEmbeddingSender, -} - -EMBEDDING_RECEIVER_FACTORIES: dict[ - EmbeddingTransferMode, Callable[[], AbstractEmbeddingReceiver] -] = { - EmbeddingTransferMode.LOCAL: LocalEmbeddingReceiver, - EmbeddingTransferMode.NIXL_WRITE: NixlWriteEmbeddingReceiver, - # [gluo FIXME] can't use pre-registered tensor as NIXL requires descriptors - # to be at matching size, need to overwrite nixl connect library - EmbeddingTransferMode.NIXL_READ: lambda: NixlReadEmbeddingReceiver(max_items=0), -} +if TYPE_CHECKING: + from collections.abc import Callable + + from dynamo.common.constants import EmbeddingTransferMode + from dynamo.common.multimodal.async_encoder_cache import AsyncEncoderCache + from dynamo.common.multimodal.embedding_transfer import ( + AbstractEmbeddingReceiver, + AbstractEmbeddingSender, + LocalEmbeddingReceiver, + LocalEmbeddingSender, + NixlReadEmbeddingReceiver, + NixlReadEmbeddingSender, + NixlWriteEmbeddingReceiver, + NixlWriteEmbeddingSender, + TransferRequest, + ) + +_EMBEDDING_TRANSFER_EXPORTS = frozenset( + { + "AbstractEmbeddingReceiver", + "AbstractEmbeddingSender", + "LocalEmbeddingReceiver", + "LocalEmbeddingSender", + "NixlReadEmbeddingReceiver", + "NixlReadEmbeddingSender", + "NixlWriteEmbeddingReceiver", + "NixlWriteEmbeddingSender", + "TransferRequest", + } +) + + +def _build_sender_factories() -> ( + dict[EmbeddingTransferMode, Callable[[], AbstractEmbeddingSender]] +): + from dynamo.common.constants import EmbeddingTransferMode + from dynamo.common.multimodal.embedding_transfer import ( + LocalEmbeddingSender, + NixlReadEmbeddingSender, + NixlWriteEmbeddingSender, + ) + + return { + EmbeddingTransferMode.LOCAL: LocalEmbeddingSender, + EmbeddingTransferMode.NIXL_WRITE: NixlWriteEmbeddingSender, + EmbeddingTransferMode.NIXL_READ: NixlReadEmbeddingSender, + } + + +def _build_receiver_factories() -> ( + dict[EmbeddingTransferMode, Callable[[], AbstractEmbeddingReceiver]] +): + from dynamo.common.constants import EmbeddingTransferMode + from dynamo.common.multimodal.embedding_transfer import ( + LocalEmbeddingReceiver, + NixlReadEmbeddingReceiver, + NixlWriteEmbeddingReceiver, + ) + + return { + EmbeddingTransferMode.LOCAL: LocalEmbeddingReceiver, + EmbeddingTransferMode.NIXL_WRITE: NixlWriteEmbeddingReceiver, + # [gluo FIXME] can't use pre-registered tensor as NIXL requires descriptors + # to be at matching size, need to overwrite nixl connect library + EmbeddingTransferMode.NIXL_READ: lambda: NixlReadEmbeddingReceiver(max_items=0), + } + + +def __getattr__(name: str) -> Any: + if name == "AsyncEncoderCache": + from dynamo.common.multimodal.async_encoder_cache import AsyncEncoderCache + + value: Any = AsyncEncoderCache + elif name in _EMBEDDING_TRANSFER_EXPORTS: + from dynamo.common.multimodal import embedding_transfer + + value = getattr(embedding_transfer, name) + elif name == "EMBEDDING_SENDER_FACTORIES": + value = _build_sender_factories() + elif name == "EMBEDDING_RECEIVER_FACTORIES": + value = _build_receiver_factories() + else: + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") + + # Cache on the module so repeat access skips this function entirely. + globals()[name] = value + return value + + +def __dir__() -> list[str]: + return sorted(__all__) + __all__ = [ "AsyncEncoderCache", diff --git a/components/src/dynamo/common/tests/multimodal/test_lazy_imports.py b/components/src/dynamo/common/tests/multimodal/test_lazy_imports.py new file mode 100644 index 000000000000..f3d030b92489 --- /dev/null +++ b/components/src/dynamo/common/tests/multimodal/test_lazy_imports.py @@ -0,0 +1,44 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""The package must not drag `torch` in behind the lightweight media loaders.""" + +from __future__ import annotations + +import subprocess +import sys + +import pytest + +pytestmark = [pytest.mark.pre_merge, pytest.mark.unit, pytest.mark.gpu_0] + +# A fresh interpreter is the only honest way to ask this: once any earlier test +# has imported torch, an eager import here would go unnoticed. +_PROBE = """ +import sys +from dynamo.common.multimodal import {names} +print("torch" in sys.modules) +""" + + +def _torch_imported_by(names: str) -> bool: + result = subprocess.run( + [sys.executable, "-c", _PROBE.format(names=names)], + capture_output=True, + text=True, + check=True, + ) + return result.stdout.strip() == "True" + + +@pytest.mark.parametrize("name", ["ImageLoader", "AudioLoader", "VideoLoader"]) +def test_media_loader_import_does_not_pull_torch(name: str) -> None: + assert not _torch_imported_by( + name + ), f"importing {name} pulled in torch; the loaders must stay usable without the ML stack" + + +def test_heavy_members_still_import() -> None: + assert _torch_imported_by( + "AsyncEncoderCache" + ), "AsyncEncoderCache should still resolve, and it legitimately needs torch" From adde1d5d8b8209eefbc69049af30872b2b86f391 Mon Sep 17 00:00:00 2001 From: ayaangazali Date: Fri, 21 Aug 2026 12:07:19 -0700 Subject: [PATCH 2/4] fix(multimodal): keep type checking exact and the probe unambiguous Review follow-up on #13626. Hide `__getattr__` from the type checker. A module-level `__getattr__` tells mypy the package may have any attribute, so a typo such as `from dynamo.common.multimodal import DoesNotExistAtAll` type-checked clean in every module importing from this package, not just here. Guarding it with `if not TYPE_CHECKING` keeps the runtime behaviour and restores typo detection, because the `TYPE_CHECKING` block already declares the real names. Declare the two factory dicts in that block as well. They were resolving to `Any`, so callers lost the key and value types. The PR description claimed the re-declarations covered every name; that was true of the classes but not of these two. Tag the probe result and parse it off its own line. `stdout.strip() == "True"` read any extra output as False, and the three loader cases assert the negative, so a stray line turned the regression this test exists to catch into a pass. Imports here really do print: `dynamo.nixl_connect` emits a CuPy fallback notice. The probe now also fails loudly if it does not find exactly one tagged answer. Bound the subprocess with `timeout=30` and mark the module `pytest.mark.timeout(60)`, per .ai/pytest-guidelines.md, so a child that hangs during import cannot hang CI. Signed-off-by: ayaangazali --- .../src/dynamo/common/multimodal/__init__.py | 55 ++++++++++++------- .../tests/multimodal/test_lazy_imports.py | 28 +++++++++- 2 files changed, 61 insertions(+), 22 deletions(-) diff --git a/components/src/dynamo/common/multimodal/__init__.py b/components/src/dynamo/common/multimodal/__init__.py index b774d5ed68c5..e710ad334865 100644 --- a/components/src/dynamo/common/multimodal/__init__.py +++ b/components/src/dynamo/common/multimodal/__init__.py @@ -36,6 +36,16 @@ TransferRequest, ) + # Declared, not assigned: the runtime values come from `__getattr__` below. + # Without these mypy widens both to `Any` and callers lose the key and value + # types. + EMBEDDING_SENDER_FACTORIES: dict[ + EmbeddingTransferMode, Callable[[], AbstractEmbeddingSender] + ] + EMBEDDING_RECEIVER_FACTORIES: dict[ + EmbeddingTransferMode, Callable[[], AbstractEmbeddingReceiver] + ] + _EMBEDDING_TRANSFER_EXPORTS = frozenset( { "AbstractEmbeddingReceiver", @@ -87,25 +97,32 @@ def _build_receiver_factories() -> ( } -def __getattr__(name: str) -> Any: - if name == "AsyncEncoderCache": - from dynamo.common.multimodal.async_encoder_cache import AsyncEncoderCache - - value: Any = AsyncEncoderCache - elif name in _EMBEDDING_TRANSFER_EXPORTS: - from dynamo.common.multimodal import embedding_transfer - - value = getattr(embedding_transfer, name) - elif name == "EMBEDDING_SENDER_FACTORIES": - value = _build_sender_factories() - elif name == "EMBEDDING_RECEIVER_FACTORIES": - value = _build_receiver_factories() - else: - raise AttributeError(f"module {__name__!r} has no attribute {name!r}") - - # Cache on the module so repeat access skips this function entirely. - globals()[name] = value - return value +# Hidden from the type checker on purpose. A module-level `__getattr__` tells +# mypy the package may have any attribute, which would silently accept a typo +# like `from dynamo.common.multimodal import DoesNotExistAtAll` in every module +# that imports from here. The `TYPE_CHECKING` block above already declares the +# real names, so the checker has what it needs without the widening. +if not TYPE_CHECKING: + + def __getattr__(name: str) -> Any: + if name == "AsyncEncoderCache": + from dynamo.common.multimodal.async_encoder_cache import AsyncEncoderCache + + value: Any = AsyncEncoderCache + elif name in _EMBEDDING_TRANSFER_EXPORTS: + from dynamo.common.multimodal import embedding_transfer + + value = getattr(embedding_transfer, name) + elif name == "EMBEDDING_SENDER_FACTORIES": + value = _build_sender_factories() + elif name == "EMBEDDING_RECEIVER_FACTORIES": + value = _build_receiver_factories() + else: + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") + + # Cache on the module so repeat access skips this function entirely. + globals()[name] = value + return value def __dir__() -> list[str]: diff --git a/components/src/dynamo/common/tests/multimodal/test_lazy_imports.py b/components/src/dynamo/common/tests/multimodal/test_lazy_imports.py index f3d030b92489..b5056ac52188 100644 --- a/components/src/dynamo/common/tests/multimodal/test_lazy_imports.py +++ b/components/src/dynamo/common/tests/multimodal/test_lazy_imports.py @@ -10,14 +10,25 @@ import pytest -pytestmark = [pytest.mark.pre_merge, pytest.mark.unit, pytest.mark.gpu_0] +pytestmark = [ + pytest.mark.pre_merge, + pytest.mark.unit, + pytest.mark.gpu_0, + pytest.mark.timeout(60), +] # A fresh interpreter is the only honest way to ask this: once any earlier test # has imported torch, an eager import here would go unnoticed. +# +# The answer is tagged and parsed off its own line rather than read from the +# whole of stdout. Imports here are free to print (a CuPy fallback notice from +# `dynamo.nixl_connect` is one that really happens), and a bare +# `stdout.strip() == "True"` would read that extra output as False, turning the +# regression this test exists to catch into a pass. _PROBE = """ import sys from dynamo.common.multimodal import {names} -print("torch" in sys.modules) +print("RESULT:" + str("torch" in sys.modules)) """ @@ -27,8 +38,19 @@ def _torch_imported_by(names: str) -> bool: capture_output=True, text=True, check=True, + timeout=30, ) - return result.stdout.strip() == "True" + answers = [ + line.removeprefix("RESULT:") + for line in result.stdout.splitlines() + if line.startswith("RESULT:") + ] + assert len(answers) == 1, ( + f"probe for {names} did not report exactly one result; " + f"stdout={result.stdout!r} stderr={result.stderr!r}" + ) + assert answers[0] in ("True", "False"), f"unexpected probe answer {answers[0]!r}" + return answers[0] == "True" @pytest.mark.parametrize("name", ["ImageLoader", "AudioLoader", "VideoLoader"]) From b70ad6d8166bedac2ab84a93dd2badf613ff95f6 Mon Sep 17 00:00:00 2001 From: ayaangazali Date: Fri, 21 Aug 2026 12:08:19 -0700 Subject: [PATCH 3/4] fix(multimodal): stop __dir__ hiding names that import fine Review follow-up on #13626. `sorted(__all__)` under-reported the package. It omitted the submodules, which `dir()` listed before this PR added a `__dir__` at all, and it omitted `AbstractEmbeddingSender` and `AbstractEmbeddingReceiver`, which `__getattr__` resolves through `_EMBEDDING_TRANSFER_EXPORTS` but which were never in `__all__`. Both import fine, so hiding them from `dir()` and REPL completion was a regression this PR introduced. Union in the lazy exports and whatever is already materialised. The lazy set is included explicitly rather than relying on `globals()`, since a name only appears there after its first access. Signed-off-by: ayaangazali --- components/src/dynamo/common/multimodal/__init__.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/components/src/dynamo/common/multimodal/__init__.py b/components/src/dynamo/common/multimodal/__init__.py index e710ad334865..cbfab9cdd238 100644 --- a/components/src/dynamo/common/multimodal/__init__.py +++ b/components/src/dynamo/common/multimodal/__init__.py @@ -126,7 +126,12 @@ def __getattr__(name: str) -> Any: def __dir__() -> list[str]: - return sorted(__all__) + # `__all__` alone under-reports: it omits the submodules, and it omits + # `AbstractEmbeddingSender` / `AbstractEmbeddingReceiver`, which resolve + # through `__getattr__` but were never public names. Union in the lazy + # exports and whatever is already materialised so `dir()` and REPL + # completion do not hide things that import fine. + return sorted(set(__all__) | _EMBEDDING_TRANSFER_EXPORTS | set(globals())) __all__ = [ From 657d2482a0c6eac3a72d82a125fb21d8c3b26138 Mon Sep 17 00:00:00 2001 From: ayaangazali Date: Mon, 24 Aug 2026 20:06:48 -0700 Subject: [PATCH 4/4] test(multimodal): surface the probe child's stderr when it dies check=True raises CalledProcessError, whose message carries the command and the exit status and nothing else. The reason the child died is in stderr, which pytest does not print, so a missing torch or PIL, or a raise during package import, reads as a bare non-zero exit with nothing to act on. Check the return code explicitly and put stderr in the assertion message. The test asserts exactly what it did before. Signed-off-by: ayaangazali Signed-off-by: ayaangazali --- .../dynamo/common/tests/multimodal/test_lazy_imports.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/components/src/dynamo/common/tests/multimodal/test_lazy_imports.py b/components/src/dynamo/common/tests/multimodal/test_lazy_imports.py index b5056ac52188..671022c01981 100644 --- a/components/src/dynamo/common/tests/multimodal/test_lazy_imports.py +++ b/components/src/dynamo/common/tests/multimodal/test_lazy_imports.py @@ -37,9 +37,15 @@ def _torch_imported_by(names: str) -> bool: [sys.executable, "-c", _PROBE.format(names=names)], capture_output=True, text=True, - check=True, timeout=30, ) + # Checked here rather than with check=True: that raises CalledProcessError, + # whose message names the command and the exit status only. The reason the + # child died is in stderr, which pytest never shows, so a missing torch or + # PIL or a raise during package import reads as a bare non-zero exit. + assert ( + result.returncode == 0 + ), f"probe for {names} exited {result.returncode}\nstderr:\n{result.stderr}" answers = [ line.removeprefix("RESULT:") for line in result.stdout.splitlines()