Skip to content

fix: handle ModelPatcher-wrapped upscale model (ComfyUI #15063) - #13

Open
cuzelac wants to merge 2 commits into
TenStrip:mainfrom
cuzelac:fix/upsampler-modelpatcher
Open

fix: handle ModelPatcher-wrapped upscale model (ComfyUI #15063)#13
cuzelac wants to merge 2 commits into
TenStrip:mainfrom
cuzelac:fix/upsampler-modelpatcher

Conversation

@cuzelac

@cuzelac cuzelac commented Jul 26, 2026

Copy link
Copy Markdown

Summary

ComfyUI is converting the latent upsampler to DynamicVRAM, which changes LATENT_UPSCALE_MODEL from a bare nn.Module into a ModelPatcher. LTX Latent Upsampler (Tiled) crashes on builds that have that change. This PR makes the node accept either shape.

Please note the version nuance before trying to reproduce — you may not hit this on a released tag (details below).

The crash

File "custom_nodes/10S_Nodes/latent_upsampler_tiled.py", line 106, in upsample_latent_tiled
    model_dtype = next(upscale_model.parameters()).dtype
AttributeError: 'ModelPatcherDynamic' object has no attribute 'parameters'

The node fails before doing any work, so there's no partial-output workaround on an affected build.

Which builds are affected

This is the part worth checking first, since it determines whether you can reproduce:

  • ComfyUI commit f8a3fd9 — "upscalers: convert latent_upsampler model to DynamicVram" (#15063, 2026-07-24) — wraps the LatentUpsampler in a CoreModelPatcher, which becomes ModelPatcherDynamic when DynamicVRAM is on. This commit is currently on ComfyUI master.
  • Tagged v0.28.3 does not contain it and still returns a bare module.

So the shape depends on the build, not cleanly on the version string. I hit the crash on a Comfy Desktop install that carried f8a3fd9 while reporting 0.28.0; after updating to v0.28.3 the same install went back to a bare module. If you test on v0.28.3 you will not see the crash — that doesn't mean it isn't real, it means the change hasn't reached your tag yet.

Separately, this isn't purely a future concern: even on builds that do have f8a3fd9, the Hunyuan 720p/1080p branches of LatentUpscaleModelLoader still return a bare module. Both shapes are reachable simultaneously.

Root cause

From comfy_extras/nodes_hunyuan.py on master, the LTX branch of the loader:

model_patcher = comfy.model_patcher.CoreModelPatcher(model, load_device=..., offload_device=...)
model.load_state_dict(sd, assign=model_patcher.is_dynamic())
model = model_patcher

A ModelPatcher is not callable and exposes no .parameters(), .to(), .cpu(), or .state_dict(). The node relied on all four, so line 106 is only the first of six break points — the others are module_size(upscale_model), upscale_model.to(device), the three upscale_model(...) forward calls, and upscale_model.cpu().

What changed

All access goes through a new _resolve_upscale_model() helper returning (module, device, dtype, load_fn, unload_fn):

  • Patcher shape: unwraps .model, uses .load_device, delegates residency to model_management.load_models_gpu.
  • Bare-module shape: unchanged behaviour — free_memory + .to(device) / .cpu().

Detection is hasattr(upscale_model, "model") and hasattr(upscale_model, "load_device"). load_device is a ModelPatcher-ism that LatentUpsampler (a plain nn.Module) doesn't carry, so it can't misfire.

The bare-module path is behaviourally identical to the current code — same dtype source, same device, same total memory reservation, same load/unload calls. No minimum ComfyUI version bump is needed, and nothing changes for users on current tags.

The tiling and cosine-window blending math is untouched.

Two things worth your attention

1. Deliberate divergence from stock on dtype. On the patcher path, patcher.model_dtype() returns None for LatentUpsampler (no get_dtype method). ComfyUI's own node passes that straight into .to(dtype=None), a no-op leaving latents at input dtype. This PR instead falls back to next(module.parameters()).dtype, preserving the bf16 cast this node has always applied to the un-normalized latents. That's intentional, not an oversight.

2. On the patcher path the model stays resident between runs. Dropping the manual .cpu() is a behaviour change beyond the crash fix — manual eviction fights the memory manager, and load_models_gpu handles offload itself. The bare-module path keeps the original explicit eviction. Happy to revisit if you'd rather force eviction in both.

Verification

Live: hit the original crash on every run, in a two-pass LTX i2v workflow (RTX 5090, PyTorch 2.10.0+cu130) on a build carrying f8a3fd9; the node completes with this patch.

Isolated, against a CoreModelPatcher wrapping a stand-in 2× upsampler:

Path Result
Patcher resolve + forward dtype=bfloat16, (1,4,2,6,6)(1,4,2,12,12)
Bare-module resolve + forward dtype=float32, correct shape
Node, tiled (6 tiles) (1,4,2,40,24)(1,4,2,80,48), weight accumulator min=max=1.0000
Node, non-tiled (1,4,2,8,8)(1,4,2,16,16)

The weight accumulator at exactly 1.0 confirms the blending math is unaffected.

Coverage caveat: the isolated test exercises ModelPatcher, not ModelPatcherDynamic — the dynamic subclass only instantiates under the full DynamicVRAM startup path. Every attribute used here is inherited unchanged from ModelPatcher, and the live run above was on ModelPatcherDynamic (that's what the traceback names), so both are covered between the two.

Commits

Split so you can take the fix without the release metadata:

  • fix(upsampler): handle ModelPatcher-wrapped upscale model — the fix, plus a compatibility note in CLAUDE.md
  • chore: bump version to 1.9.6__init__.py, pyproject.toml, README version history

Drop the second commit if you'd rather assign the version yourself; I have no attachment to 1.9.6.

cuzelac and others added 2 commits July 26, 2026 01:49
- ComfyUI 0.28.0 wraps LatentUpsampler in a ModelPatcher, which is not
  callable and exposes no .parameters()/.to()/.cpu()
- resolve the handle via _resolve_upscale_model(); legacy bare-module
  inputs still work
- delegate residency to load_models_gpu instead of manual .to()/.cpu(),
  which fought the memory manager
- model_dtype() returns None for LatentUpsampler, so fall back to
  parameter dtype to preserve the bf16 cast

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
@sho81

sho81 commented Jul 27, 2026

Copy link
Copy Markdown

Confirmed for me / by me: git checkout fix/upsampler-modelpatcher in the forked repo solved the problem for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants