From cd4465dfa4d62416accc3837172fedfa57a778e2 Mon Sep 17 00:00:00 2001 From: Ken Simpson Date: Fri, 17 Apr 2026 16:31:42 -0700 Subject: [PATCH] feat: configure modal gpu type via env --- README.md | 4 ++++ comfyui_modal_sync_cloud.py | 2 +- docs/LEARNINGS.md | 1 + settings.py | 2 ++ tests/test_modal_executor_node.py | 4 ++++ tests/test_settings.py | 14 ++++++++++++++ 6 files changed, 26 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index b9408ec..ec24379 100644 --- a/README.md +++ b/README.md @@ -101,6 +101,7 @@ Important distinction: - the Modal image now installs the core ComfyUI runtime Python packages automatically, including packages ComfyUI and custom/API nodes commonly import at startup such as `alembic`, `pydantic-settings`, `spandrel`, `kornia`, and `comfy-kitchen` - the Modal image now includes `comfy-kitchen>=0.2.7` because ComfyUI's FP8/FP4 weight-loading path crashes with `NoneType.Params` if quantized layout support is missing - the Modal image now pins `torch==2.10.0`, `torchvision==0.25.0`, and `torchaudio==2.10.0` from PyTorch's CUDA 12.8 wheel index instead of floating `latest`, which keeps the remote runtime aligned with the local ComfyUI `+cu128` stack and avoids newer wheel/driver mismatches +- the deployed remote class now takes its GPU type from `COMFY_MODAL_GPU` instead of hardcoding `A100`, so you can choose Modal GPU classes such as `L40S`, `A100`, or any other Modal-supported GPU string before app creation - the deployed remote class now enables Modal memory snapshots by default so later cold starts can skip more initialization work after the first deployed invocations; GPU memory snapshots remain opt-in because they are still an alpha feature in Modal - the deployed remote class now defaults to a `scaledown_window` of 600 seconds with `min_containers=0`, which keeps recently used containers warm for a while without forcing a permanently warm GPU - the Modal image filter now preserves internal ComfyUI Python packages such as `comfy/ldm/models` while still excluding top-level runtime asset folders like `models/` and `output/` @@ -235,9 +236,12 @@ These environment variables are supported: - `COMFY_MODAL_ALLOW_EPHEMERAL_FALLBACK`: Re-enable slow `app.run()` fallback in remote mode. Default: `false`. - `COMFY_MODAL_ENABLE_MEMORY_SNAPSHOT`: Enable Modal CPU memory snapshots for the deployed remote class. Default: `true`. - `COMFY_MODAL_ENABLE_GPU_MEMORY_SNAPSHOT`: Opt into Modal GPU memory snapshots. Default: `false`. +- `COMFY_MODAL_GPU`: Modal GPU type to request when deploying the remote class. Default: `A100`. - `COMFY_MODAL_SCALEDOWN_WINDOW`: Keep idle Modal containers warm for this many seconds after a request. Default: `600`. - `COMFY_MODAL_MIN_CONTAINERS`: Keep at least this many containers warm at all times. Default: `0`. +If you change `COMFY_MODAL_GPU`, redeploy the Modal app or delete it and let first-run auto-deploy recreate it. Modal deployment hardware is fixed at deploy time, so changing the environment variable locally does not mutate an already-deployed app in place. + ## Development - Run tests with `uv run pytest`. diff --git a/comfyui_modal_sync_cloud.py b/comfyui_modal_sync_cloud.py index 34523e8..6df3d93 100644 --- a/comfyui_modal_sync_cloud.py +++ b/comfyui_modal_sync_cloud.py @@ -870,7 +870,7 @@ def _prewarm_restored_runtime() -> None: def _remote_engine_cls_options(settings: Any, vol: Any, image: Any) -> dict[str, Any]: """Build the Modal class options for the deployed remote execution runtime.""" options: dict[str, Any] = { - "gpu": "A100", + "gpu": settings.modal_gpu, "volumes": {settings.remote_storage_root: vol}, "scaledown_window": settings.scaledown_window_seconds, "min_containers": settings.min_containers, diff --git a/docs/LEARNINGS.md b/docs/LEARNINGS.md index b08bd09..5eba05e 100644 --- a/docs/LEARNINGS.md +++ b/docs/LEARNINGS.md @@ -43,4 +43,5 @@ - Modal volume SDK calls should not run directly on the aiohttp request event loop. Running them in a worker thread avoids `AsyncUsageWarning`, and a missing path from `Volume.listdir()` should be treated as an ordinary "not uploaded yet" cache miss instead of a hard failure. - The Modal container can pick up a non-package `utils` module before ComfyUI initializes. Preloading ComfyUI's own top-level `utils` package from the bundled source tree prevents `utils.install_util` import failures during `init_extra_nodes(...)`. - If the goal is "no idle billing, but no minute-long rebuild every run", the right Modal pattern is a deployed app with `min_containers=0`, not ephemeral `app.run()`. That lets Modal reuse the built image and deployed objects while still scaling compute down to zero between executions. +- Modal hardware selection belongs in deployment configuration, not in request payloads. For this repo, the remote class should read a configurable GPU string from environment at deploy time, and changing it later requires a redeploy or app recreation because existing deployed app hardware does not update in place. - Modal memory snapshots only help once the app is deployed. They are a good default for CPU-side initialization work, while GPU memory snapshots should remain opt-in because Modal still labels them alpha and they may require additional runtime refactors. diff --git a/settings.py b/settings.py index 5947fcf..4172b0d 100644 --- a/settings.py +++ b/settings.py @@ -30,6 +30,7 @@ class ModalSyncSettings: custom_nodes_archive_name: str comfyui_root: Path | None custom_nodes_dir: Path | None + modal_gpu: str = "A100" scaledown_window_seconds: int = 600 min_containers: int = 0 @@ -150,6 +151,7 @@ def get_settings() -> ModalSyncSettings: ), comfyui_root=comfyui_root, custom_nodes_dir=custom_nodes_dir, + modal_gpu=os.getenv("COMFY_MODAL_GPU", "A100").strip() or "A100", scaledown_window_seconds=_read_int_env("COMFY_MODAL_SCALEDOWN_WINDOW", 600), min_containers=_read_int_env("COMFY_MODAL_MIN_CONTAINERS", 0), ) diff --git a/tests/test_modal_executor_node.py b/tests/test_modal_executor_node.py index 0fd7357..1573b12 100644 --- a/tests/test_modal_executor_node.py +++ b/tests/test_modal_executor_node.py @@ -348,6 +348,7 @@ def test_modal_cloud_builds_snapshot_enabled_cls_options( remote_storage_root="/storage", enable_memory_snapshot=True, enable_gpu_memory_snapshot=False, + modal_gpu="L40S", scaledown_window_seconds=600, min_containers=0, ) @@ -356,6 +357,7 @@ def test_modal_cloud_builds_snapshot_enabled_cls_options( assert options["enable_memory_snapshot"] is True assert "experimental_options" not in options + assert options["gpu"] == "L40S" assert options["volumes"] == {"/storage": "volume"} assert options["scaledown_window"] == 600 assert options["min_containers"] == 0 @@ -364,6 +366,7 @@ def test_modal_cloud_builds_snapshot_enabled_cls_options( remote_storage_root="/storage", enable_memory_snapshot=True, enable_gpu_memory_snapshot=True, + modal_gpu="A100", scaledown_window_seconds=900, min_containers=1, ) @@ -373,6 +376,7 @@ def test_modal_cloud_builds_snapshot_enabled_cls_options( "image", ) assert gpu_snapshot_options["experimental_options"] == {"enable_gpu_snapshot": True} + assert gpu_snapshot_options["gpu"] == "A100" assert gpu_snapshot_options["scaledown_window"] == 900 assert gpu_snapshot_options["min_containers"] == 1 diff --git a/tests/test_settings.py b/tests/test_settings.py index b3fcd25..7545ef7 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -43,3 +43,17 @@ def test_settings_prefers_modal_specific_comfyui_root_env( assert resolved == env_root.resolve() + +def test_settings_reads_modal_gpu_override( + settings_module: Any, + monkeypatch: Any, +) -> None: + """The Modal GPU type should be configurable via environment variable.""" + monkeypatch.setenv("COMFY_MODAL_GPU", "L40S") + settings_module.get_settings.cache_clear() + try: + settings = settings_module.get_settings() + finally: + settings_module.get_settings.cache_clear() + + assert settings.modal_gpu == "L40S"