Add SeedVR2 support (CORE-6)#14110
Open
pollockjj wants to merge 10 commits into
Open
Conversation
pollockjj
requested review from
Kosinkadink,
alexisrolland,
comfyanonymous,
guill,
kijai and
rattus128
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds first-class SeedVR2 support across ComfyUI’s model detection, latent/conditioning conventions, and VAE/attention/RoPE execution paths, with extensive regression coverage to lock down SeedVR2-specific invariants.
Changes:
- Add SeedVR2 model detection and a
supported_models.SeedVR2integration (latent format + dtype policy). - Add SeedVR2-specific VAE encode/decode tiling paths, memory estimation, and node-level controls (including “0 disables temporal slicing” behavior for SeedVR2).
- Add broad regression coverage for attention backends, RoPE rewrite invariants, VAE boundary behavior, and static-audit constraints.
Reviewed changes
Copilot reviewed 56 out of 58 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests-unit/comfy_test/test_var_attention_pytorch_seedvr2_guard.py | Regression coverage for SeedVR2-specific nested-tensor guard behavior in var-attention. |
| tests-unit/comfy_test/test_vae_encode_tiled_seedvr2_method.py | Ensures VAE.encode_tiled_seedvr2 exists, delegates correctly, and respects tiling parameters. |
| tests-unit/comfy_test/test_vae_encode_tiled_fallback_dispatcher_seedvr2.py | Verifies OOM fallback routes SeedVR2 vs generic 3D encode tiler correctly. |
| tests-unit/comfy_test/test_vae_encode_tiled_explicit_dispatcher_seedvr2.py | Verifies explicit encode_tiled dispatcher routes SeedVR2 vs generic tiler correctly. |
| tests-unit/comfy_test/test_vae_decode_tiled_dispatcher_seedvr2_4d.py | Regression for 4D collapsed SeedVR2 latent routing to decode_tiled_seedvr2. |
| tests-unit/comfy_test/test_seedvr2_windows_static_verify.py | Windows static-token contract verification for SeedVR2-related codepaths. |
| tests-unit/comfy_test/test_seedvr2_vae_graph_boundaries.py | Boundary/contract tests ensuring no hidden state and stable latent contracts. |
| tests-unit/comfy_test/test_seedvr2_saved_latent_decode_boundary.py | Ensures saved/loaded SeedVR2 latents decode without re-running preprocessing. |
| tests-unit/comfy_test/test_seedvr2_resize_and_pad_pre_encode_state.py | Tests SeedVR2 resize/pad behavior and schema contracts. |
| tests-unit/comfy_test/test_seedvr2_refactor_nodes.py | Regression tests for node behavior and metadata boundaries around SeedVR2. |
| tests-unit/comfy_test/test_seedvr2_non_goal_static_audit.py | Optional (env-gated) static audit ensuring forbidden “non-goal” files aren’t dirty. |
| tests-unit/comfy_test/test_seedvr2_hidden_state_static_audit.py | AST-based audit ensuring no forbidden hidden-state fields/strings are used. |
| tests-unit/comfy_test/test_seedvr2_dtype.py | Regression coverage for dtype/device inference and several SeedVR2-specific math behaviors. |
| tests-unit/comfy_test/test_seedvr_var_attention_backends.py | Tests attention backend selection/rebinding and backend-specific contracts. |
| tests-unit/comfy_test/test_seedvr_vae_tiled_temporal_slicing.py | Tests tiled VAE temporal slicing contracts and overlap propagation. |
| tests-unit/comfy_test/test_seedvr_vae_tiled_encode_runt_slice_override.py | Ensures encode temporal slicing merges runt tails and preserves min-size on failure. |
| tests-unit/comfy_test/test_seedvr_vae_tiled_decode_latent_min_size_override.py | Ensures decode slicing can be disabled per-call and preserves min-size on failure. |
| tests-unit/comfy_test/test_seedvr_vae_tiled_decode_5d.py | Validates JSON probe contract for real tiled decode verification payloads. |
| tests-unit/comfy_test/test_seedvr_vae_tiled_args_no_mutate.py | Static check that tiled_args hidden state is not used in SeedVR VAE code. |
| tests-unit/comfy_test/test_seedvr_vae_loader_metadata.py | Loader regression: SeedVR2 magic-key detection sets correct VAE metadata. |
| tests-unit/comfy_test/test_seedvr_vae_decode_unpadded_t.py | Ensures temporal padding/trim behaviors match expected post-processing. |
| tests-unit/comfy_test/test_seedvr_vae_decode_guards.py | Wrapper decode input-shape validation tests (4D collapsed vs 5D). |
| tests-unit/comfy_test/test_seedvr_vae_decode_batch_axes.py | Verifies decode preserves batch/time axes across variants and tiling. |
| tests-unit/comfy_test/test_seedvr_vae_attention_fence.py | Ensures SeedVR VAE self-attention does not route through global optimized attention. |
| tests-unit/comfy_test/test_seedvr_vae_5d_tiled_decode.py | Extensive SeedVR2 tiled decode behavior and node temporal-control behavior tests. |
| tests-unit/comfy_test/test_seedvr_rope_rewrite.py | Regression suite pinning SeedVR2 RoPE rewrite invariants and output equality. |
| tests-unit/comfy_test/test_seedvr_rope_delegation.py | Ensures apply_rotary_emb delegates to apply_rope1 and remains output-identical. |
| tests-unit/comfy_test/test_seedvr_latent_format.py | Validates SeedVR2 latent format channel behavior and empty-latent preservation. |
| tests-unit/comfy_test/test_seedvr_groupnorm_limit.py | Regression for GroupNorm memory-limit gating behavior in SeedVR VAE. |
| tests-unit/comfy_test/test_seedvr_forward_no_device_cast.py | Ensures forward paths avoid get_torch_device() calls (device-cast avoidance). |
| tests-unit/comfy_test/test_seedvr_clear_vae_memory_soft_empty_cache.py | Ensures cache clear uses model_management.soft_empty_cache not torch.cuda.empty_cache. |
| tests-unit/comfy_test/test_seedvr_7b_final_block_text_path.py | Regression tests for 3B/7B behavior differences (final block path & rope types). |
| tests-unit/comfy_test/test_diffusers_metadata_guard.py | Regression for diffusers-format VAE conversion guard with missing metadata key. |
| tests-unit/comfy_test/seedvr_vae_wrapper_forward_test.py | Ensures wrapper forward returns the correct tensor triple and avoids .sample access. |
| tests-unit/comfy_test/seedvr_vae_forward_test.py | Ensures SeedVR VAE forward respects tensor/tuple returns and avoids diffusers attrs. |
| tests-unit/comfy_test/seedvr_model_test.py | Conditioning split hardening tests and “no bare except” AST pinning. |
| tests-unit/comfy_test/model_detection_test.py | Adds SeedVR2 model detection test coverage for 3B/7B key patterns. |
| tests-unit/comfy_extras_test/test_seedvr2_node_boundaries.py | Ensures SeedVR2 resize nodes stay preprocess-only and don’t call encode/decode. |
| tests-unit/comfy_extras_test/test_seedvr_node_signature.py | Schema vs execute() signature ordering regression test for SeedVR2 resize nodes. |
| nodes.py | Adds SeedVR2 temporal controls (min=0) and loads nodes_seedvr.py as a builtin extra node. |
| comfy/supported_models.py | Adds SeedVR2 supported model config, dtype policy, and registration. |
| comfy/supported_models_base.py | Extends set_inference_dtype signature to accept optional device. |
| comfy/sd.py | SeedVR2 VAE detection/loader path, SeedVR2 tiling encode/decode, memory estimation, and dtype-device dispatch helper. |
| comfy/sample.py | Preserves empty latent channel multiples for SeedVR2 collapsed (16*T) channel layout. |
| comfy/model_detection.py | Adds UNet config detection rules for SeedVR2 3B/7B variants. |
| comfy/model_base.py | Adds a SeedVR2 BaseModel wrapper routing to comfy.ldm.seedvr.model.NaDiT. |
| comfy/ldm/modules/diffusionmodules/model.py | Extends timestep embedding function to support flip/shift args (diffusers-aligned). |
| comfy/latent_formats.py | Adds SeedVR2 latent format and new preserve_empty_channel_multiples capability. |
| .gitignore | Ignores .pyisolate_venvs/. |
| .github/workflows/test-unit.yml | Adjusts branch triggers (adds develop) and removes continue-on-error. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
341
to
+346
| temporal_compression = vae.temporal_compression_decode() | ||
| if temporal_compression is not None: | ||
| temporal_size = max(2, temporal_size // temporal_compression) | ||
| temporal_overlap = max(1, min(temporal_size // 2, temporal_overlap // temporal_compression)) | ||
| if temporal_size <= 0: | ||
| temporal_size = 0 | ||
| temporal_overlap = 0 | ||
| else: |
|
Caution Review failedFailed to post review comments Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces SeedVR2, a video-text diffusion model, along with supporting infrastructure. It implements the NaDiT transformer backbone with variable-length attention, a video VAE with temporal/spatial tiling, model detection for multiple checkpoint variants, and a complete ComfyUI extension with nodes for input preprocessing, output postprocessing, text conditioning, and progressive sampling. The implementation includes comprehensive test coverage for model detection, node contracts, tensor transformations, memory estimation, and internal regression scenarios. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
comfy/ldm/seedvr/model.py (1)
1382-1388: 💤 Low valueIn-place operations may cause autograd issues during training.
The
mul_andadd_in-place operations work for inference, but if training/finetuning is later needed, these can trigger "variable modified by inplace operation" errors. Other parts of this file (e.g.,_apply_rope1_partialat line 657) explicitly clone tensors whenrequires_gradis True.This is fine for the current inference-focused use case, but worth noting if training support is added later.
🤖 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 `@comfy/ldm/seedvr/model.py` around lines 1382 - 1388, The in-place ops on hid (mul_ and add_) can break autograd; change them to out-of-place ops or clone when gradients are required: e.g., ensure in the mode handling code that either you replace hid.mul_(...) and .add_(...) with hid.mul(...).add(...) (and similarly for hid.mul_(gateA...) → hid.mul(gateA...)), or first do hid = hid.clone() if hid.requires_grad before performing the in-place ops; refer to the variables hid, scaleA, scaleB, shiftA, shiftB, gateA, gateB in the mode == "in"/"out" block to locate and update the code.tests-unit/comfy_test/test_seedvr_vae_tiled_args_no_mutate.py (1)
5-11: ⚡ Quick winNarrow the static check to
VideoAutoencoderKLWrapper.decodeonly.This regex scans the whole file, so unrelated
tiled_argsreferences can break this test even whendecodeis clean. Scope the assertion to thedecodemethod AST/body to avoid false failures.🔧 Proposed refinement
import re +import ast from pathlib import Path def test_seedvr_vae_decode_uses_explicit_tiling_options_not_object_state(): path = Path(__file__).resolve().parents[2] / "comfy" / "ldm" / "seedvr" / "vae.py" src = path.read_text(encoding="utf-8") - assert not re.search(r"(?:self\.)?tiled_args\b", src), ( - "VideoAutoencoderKLWrapper.decode must not read or mutate tiled_args " - f"object state. Source path: {path}" - ) + tree = ast.parse(src) + decode_fn = None + for node in tree.body: + if isinstance(node, ast.ClassDef) and node.name == "VideoAutoencoderKLWrapper": + for member in node.body: + if isinstance(member, ast.FunctionDef) and member.name == "decode": + decode_fn = member + break + break + + assert decode_fn is not None, f"Could not locate VideoAutoencoderKLWrapper.decode in {path}" + + has_tiled_args_ref = any( + ( + isinstance(n, ast.Name) and n.id == "tiled_args" + ) or ( + isinstance(n, ast.Attribute) + and isinstance(n.value, ast.Name) + and n.value.id == "self" + and n.attr == "tiled_args" + ) + for n in ast.walk(decode_fn) + ) + assert not has_tiled_args_ref, ( + "VideoAutoencoderKLWrapper.decode must not read or mutate tiled_args object state. " + f"Source path: {path}" + )🤖 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-unit/comfy_test/test_seedvr_vae_tiled_args_no_mutate.py` around lines 5 - 11, The test currently searches the whole file for "tiled_args" which causes false positives; change it to parse the file AST, locate the class VideoAutoencoderKLWrapper and then the decode method body (FunctionDef named "decode") and assert that no Name or Attribute nodes refer to "tiled_args" inside that FunctionDef only; implement this by using ast.parse(src), finding the ClassDef "VideoAutoencoderKLWrapper", finding its FunctionDef "decode", then either inspect its .body AST nodes for ast.Name/ast.Attribute with id/attr "tiled_args" or extract the decode source segment and run the regex against that segment instead of the whole file.comfy/ldm/seedvr/vae.py (3)
617-617: 💤 Low valueComment in French; prefer English for consistency.
The rest of the codebase uses English comments. Consider translating: "For 'nearest' and other compatible modes, no fix needed."
🤖 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 `@comfy/ldm/seedvr/vae.py` at line 617, Replace the French inline comment "Pour 'nearest' et autres modes compatibles, pas de fix nécessaire" with an English equivalent (e.g., "For 'nearest' and other compatible modes, no fix needed") in the same location in comfy/ldm/seedvr/vae.py so comments match the rest of the codebase; keep the meaning unchanged and maintain comment formatting around the surrounding code block.
274-274: ⚡ Quick winBare assert provides no diagnostic context on failure.
If
output_len <= 0due to edge-case inputs (e.g., very smallinput_lenrelative to kernel/stride), this assert gives no helpful information. Consider raising aValueErrorwith context.Proposed fix
- assert output_len > 0 + if output_len <= 0: + raise ValueError( + f"get_cache_size: computed output_len={output_len} is non-positive " + f"(input_len={input_len}, pad_len={pad_len}, kernel_size={conv_module.kernel_size[dim]}, " + f"stride={conv_module.stride[dim]}, dilation={conv_module.dilation[dim]})" + )🤖 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 `@comfy/ldm/seedvr/vae.py` at line 274, Replace the bare assertion "assert output_len > 0" with an explicit ValueError that includes diagnostic context; raise ValueError when output_len <= 0 and include relevant variables (e.g., input_len, kernel, stride, output_len) and a short message so callers can see why computation in the VAE path failed — locate the check around "output_len" in comfy/ldm/seedvr/vae.py (the block that currently does "assert output_len > 0") and change it to raise a ValueError with those details.
266-266: 💤 Low valueMinor typo in variable name.
dilated_kernerl_size→dilated_kernel_size. While functional, this typo could cause confusion during debugging or maintenance.Proposed fix
- dilated_kernerl_size = conv_module.dilation[dim] * (conv_module.kernel_size[dim] - 1) + 1 - output_len = (input_len + pad_len - dilated_kernerl_size) // conv_module.stride[dim] + 1 + dilated_kernel_size = conv_module.dilation[dim] * (conv_module.kernel_size[dim] - 1) + 1 + output_len = (input_len + pad_len - dilated_kernel_size) // conv_module.stride[dim] + 1 remain_len = ( - input_len + pad_len - ((output_len - 1) * conv_module.stride[dim] + dilated_kernerl_size) + input_len + pad_len - ((output_len - 1) * conv_module.stride[dim] + dilated_kernel_size) ) - overlap_len = dilated_kernerl_size - conv_module.stride[dim] + overlap_len = dilated_kernel_size - conv_module.stride[dim]🤖 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 `@comfy/ldm/seedvr/vae.py` at line 266, Rename the misspelled local variable dilated_kernerl_size to dilated_kernel_size where it is computed from conv_module.dilation and conv_module.kernel_size (the assignment using conv_module.dilation[dim] * (conv_module.kernel_size[dim] - 1) + 1); update any subsequent references in the same scope (e.g., any later uses within the same function or method) to the new name to avoid NameError and reduce confusion.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@comfy/ldm/seedvr/vae.py`:
- Line 938: Constructor default for the interpolate flag contradicts the usage
assert (assert not self.interpolate) — change the default in the class/method
initializer that sets self.interpolate (the constructor near line 891) from True
to False so it matches the implementation constraint, or if you prefer runtime
flexibility remove the assert and handle both modes explicitly in the methods
that rely on interpolate (e.g., the code path used by UpDecoderBlock3D); update
the constructor signature that defines interpolate and any callers that rely on
the default to preserve consistent behavior.
In `@tests-unit/comfy_test/test_seedvr_clear_vae_memory_soft_empty_cache.py`:
- Around line 26-31: The test sets global CLI state by assigning `_cli_args.cpu
= True` at import time, which can leak into other tests; wrap this change in a
module-scoped pytest fixture (e.g., `restore_cli_args`) that captures the
original `_cli_args.cpu` value, sets `_cli_args.cpu = True` for the test/module,
yields, and then restores the original value afterwards so `_cli_args` is not
permanently mutated; reference `_cli_args` from `comfy.cli_args` and name the
fixture `restore_cli_args` (autouse or module-scoped) to locate where to apply
the change.
In `@tests-unit/comfy_test/test_seedvr2_non_goal_static_audit.py`:
- Around line 22-39: The test currently calls _git_changed_paths() which runs
"git diff" against the working tree/index (empty in CI), so committed PR changes
are missed; update _git_changed_paths to compute the PR base (e.g. run "git
merge-base origin/main HEAD" or fallback to "origin/main") and then run "git
diff --name-only <base>..HEAD" to list committed changes (keep the existing
cached/working behavior optional), handle subprocess errors by skipping the test
as before, and ensure test_seedvr2_non_goal_files_are_not_dirty uses this
updated helper so committed changes in the PR are detected.
---
Nitpick comments:
In `@comfy/ldm/seedvr/model.py`:
- Around line 1382-1388: The in-place ops on hid (mul_ and add_) can break
autograd; change them to out-of-place ops or clone when gradients are required:
e.g., ensure in the mode handling code that either you replace hid.mul_(...) and
.add_(...) with hid.mul(...).add(...) (and similarly for hid.mul_(gateA...) →
hid.mul(gateA...)), or first do hid = hid.clone() if hid.requires_grad before
performing the in-place ops; refer to the variables hid, scaleA, scaleB, shiftA,
shiftB, gateA, gateB in the mode == "in"/"out" block to locate and update the
code.
In `@comfy/ldm/seedvr/vae.py`:
- Line 617: Replace the French inline comment "Pour 'nearest' et autres modes
compatibles, pas de fix nécessaire" with an English equivalent (e.g., "For
'nearest' and other compatible modes, no fix needed") in the same location in
comfy/ldm/seedvr/vae.py so comments match the rest of the codebase; keep the
meaning unchanged and maintain comment formatting around the surrounding code
block.
- Line 274: Replace the bare assertion "assert output_len > 0" with an explicit
ValueError that includes diagnostic context; raise ValueError when output_len <=
0 and include relevant variables (e.g., input_len, kernel, stride, output_len)
and a short message so callers can see why computation in the VAE path failed —
locate the check around "output_len" in comfy/ldm/seedvr/vae.py (the block that
currently does "assert output_len > 0") and change it to raise a ValueError with
those details.
- Line 266: Rename the misspelled local variable dilated_kernerl_size to
dilated_kernel_size where it is computed from conv_module.dilation and
conv_module.kernel_size (the assignment using conv_module.dilation[dim] *
(conv_module.kernel_size[dim] - 1) + 1); update any subsequent references in the
same scope (e.g., any later uses within the same function or method) to the new
name to avoid NameError and reduce confusion.
In `@tests-unit/comfy_test/test_seedvr_vae_tiled_args_no_mutate.py`:
- Around line 5-11: The test currently searches the whole file for "tiled_args"
which causes false positives; change it to parse the file AST, locate the class
VideoAutoencoderKLWrapper and then the decode method body (FunctionDef named
"decode") and assert that no Name or Attribute nodes refer to "tiled_args"
inside that FunctionDef only; implement this by using ast.parse(src), finding
the ClassDef "VideoAutoencoderKLWrapper", finding its FunctionDef "decode", then
either inspect its .body AST nodes for ast.Name/ast.Attribute with id/attr
"tiled_args" or extract the decode source segment and run the regex against that
segment instead of the whole file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ab690dd6-37ba-4fdd-be26-fe5f45f4eac6
📒 Files selected for processing (58)
.github/workflows/test-unit.yml.gitignorecomfy/latent_formats.pycomfy/ldm/modules/attention.pycomfy/ldm/modules/diffusionmodules/model.pycomfy/ldm/seedvr/model.pycomfy/ldm/seedvr/vae.pycomfy/model_base.pycomfy/model_detection.pycomfy/sample.pycomfy/samplers.pycomfy/sd.pycomfy/supported_models.pycomfy/supported_models_base.pycomfy_extras/nodes_seedvr.pynodes.pytests-unit/comfy_extras_test/test_seedvr2_node_boundaries.pytests-unit/comfy_extras_test/test_seedvr2_post_processing.pytests-unit/comfy_extras_test/test_seedvr_conditioning_hardening.pytests-unit/comfy_extras_test/test_seedvr_node_signature.pytests-unit/comfy_test/model_detection_test.pytests-unit/comfy_test/seedvr_model_test.pytests-unit/comfy_test/seedvr_vae_forward_test.pytests-unit/comfy_test/seedvr_vae_wrapper_forward_test.pytests-unit/comfy_test/test_diffusers_metadata_guard.pytests-unit/comfy_test/test_seedvr2_dtype.pytests-unit/comfy_test/test_seedvr2_hidden_state_static_audit.pytests-unit/comfy_test/test_seedvr2_non_goal_static_audit.pytests-unit/comfy_test/test_seedvr2_refactor_nodes.pytests-unit/comfy_test/test_seedvr2_resize_and_pad_pre_encode_state.pytests-unit/comfy_test/test_seedvr2_saved_latent_decode_boundary.pytests-unit/comfy_test/test_seedvr2_vae_graph_boundaries.pytests-unit/comfy_test/test_seedvr2_windows_static_verify.pytests-unit/comfy_test/test_seedvr_7b_final_block_text_path.pytests-unit/comfy_test/test_seedvr_clear_vae_memory_soft_empty_cache.pytests-unit/comfy_test/test_seedvr_forward_no_device_cast.pytests-unit/comfy_test/test_seedvr_groupnorm_limit.pytests-unit/comfy_test/test_seedvr_latent_format.pytests-unit/comfy_test/test_seedvr_progressive_sampler.pytests-unit/comfy_test/test_seedvr_rope_delegation.pytests-unit/comfy_test/test_seedvr_rope_rewrite.pytests-unit/comfy_test/test_seedvr_vae_5d_tiled_decode.pytests-unit/comfy_test/test_seedvr_vae_attention_fence.pytests-unit/comfy_test/test_seedvr_vae_decode_batch_axes.pytests-unit/comfy_test/test_seedvr_vae_decode_guards.pytests-unit/comfy_test/test_seedvr_vae_decode_unpadded_t.pytests-unit/comfy_test/test_seedvr_vae_loader_metadata.pytests-unit/comfy_test/test_seedvr_vae_tiled_args_no_mutate.pytests-unit/comfy_test/test_seedvr_vae_tiled_decode_5d.pytests-unit/comfy_test/test_seedvr_vae_tiled_decode_latent_min_size_override.pytests-unit/comfy_test/test_seedvr_vae_tiled_encode_runt_slice_override.pytests-unit/comfy_test/test_seedvr_vae_tiled_temporal_slicing.pytests-unit/comfy_test/test_seedvr_var_attention_backends.pytests-unit/comfy_test/test_vae_decode_tiled_dispatcher_seedvr2_4d.pytests-unit/comfy_test/test_vae_encode_tiled_explicit_dispatcher_seedvr2.pytests-unit/comfy_test/test_vae_encode_tiled_fallback_dispatcher_seedvr2.pytests-unit/comfy_test/test_vae_encode_tiled_seedvr2_method.pytests-unit/comfy_test/test_var_attention_pytorch_seedvr2_guard.py
| self.spatial_ratio = 2 if spatial_up else 1 | ||
| self.slicing = slicing | ||
|
|
||
| assert not self.interpolate |
There was a problem hiding this comment.
Assert contradicts constructor's default parameter.
Line 891 defaults interpolate=True, but line 938 asserts not self.interpolate. While current callers (e.g., UpDecoderBlock3D) explicitly pass interpolate=False, the default parameter is misleading and could cause runtime failures for future callers.
Proposed fix: align default with the implementation constraint
def __init__(
self,
channels,
out_channels = None,
inflation_mode = "tail",
temporal_up: bool = False,
spatial_up: bool = True,
slicing: bool = False,
- interpolate = True,
+ interpolate = False,
name: str = "conv",🤖 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 `@comfy/ldm/seedvr/vae.py` at line 938, Constructor default for the interpolate
flag contradicts the usage assert (assert not self.interpolate) — change the
default in the class/method initializer that sets self.interpolate (the
constructor near line 891) from True to False so it matches the implementation
constraint, or if you prefer runtime flexibility remove the assert and handle
both modes explicitly in the methods that rely on interpolate (e.g., the code
path used by UpDecoderBlock3D); update the constructor signature that defines
interpolate and any callers that rely on the default to preserve consistent
behavior.
Comment on lines
+22
to
+39
| def _git_changed_paths(*args): | ||
| result = subprocess.run( | ||
| ["git", "-C", str(ROOT), "diff", "--name-only", *args], | ||
| text=True, | ||
| capture_output=True, | ||
| check=False, | ||
| ) | ||
| if result.returncode != 0: | ||
| pytest.skip(f"git diff unavailable: {result.stderr.strip()}") | ||
| return set(result.stdout.splitlines()) | ||
|
|
||
|
|
||
| def test_seedvr2_non_goal_files_are_not_dirty(): | ||
| changed = _git_changed_paths() | ||
| changed.update(_git_changed_paths("--cached")) | ||
| changed_forbidden = sorted(FORBIDDEN_FILES.intersection(changed)) | ||
| if changed_forbidden: | ||
| pytest.fail(f"forbidden non-goal files changed: {changed_forbidden}") |
There was a problem hiding this comment.
Audit misses committed PR changes on clean checkouts.
Line 24/Line 36 use git diff against working tree/index only, which is usually empty in CI after checkout. That means forbidden files changed in the PR can slip through undetected.
Suggested fix
def _git_changed_paths(*args):
result = subprocess.run(
["git", "-C", str(ROOT), "diff", "--name-only", *args],
@@
def test_seedvr2_non_goal_files_are_not_dirty():
- changed = _git_changed_paths()
- changed.update(_git_changed_paths("--cached"))
+ base_ref = os.environ.get("GITHUB_BASE_REF")
+ if base_ref:
+ changed = _git_changed_paths(f"origin/{base_ref}...HEAD")
+ else:
+ # local fallback when base ref is unavailable
+ changed = _git_changed_paths("HEAD~1...HEAD")
changed_forbidden = sorted(FORBIDDEN_FILES.intersection(changed))
if changed_forbidden:
pytest.fail(f"forbidden non-goal files changed: {changed_forbidden}")🤖 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-unit/comfy_test/test_seedvr2_non_goal_static_audit.py` around lines 22
- 39, The test currently calls _git_changed_paths() which runs "git diff"
against the working tree/index (empty in CI), so committed PR changes are
missed; update _git_changed_paths to compute the PR base (e.g. run "git
merge-base origin/main HEAD" or fallback to "origin/main") and then run "git
diff --name-only <base>..HEAD" to list committed changes (keep the existing
cached/working behavior optional), handle subprocess errors by skipping the test
as before, and ensure test_seedvr2_non_goal_files_are_not_dirty uses this
updated helper so committed changes in the PR are detected.
|
seems it doesn't work with the Numz files. https://huggingface.co/numz/SeedVR2_comfyUI Also, the bundled checkpoint here, it means the original file from ByteDance? The size is pretty big for my poor nvme, 33GB for 7b models. Can we get the comfy file somehow, or should we wait until it gets merged? |
Author
Updated PR with interim models location. |
Thank you! The first try worked fine, and I come here to asking about the VAE process (decode/encode) not utilizing dynamic VRAM. But now, after restarting Comfy to test it again, I got this error G:\AI\ComfyUI>git show
commit f632ec67da9f86056ff7fa95cd79a677f840296b (HEAD -> seedvr-test)
Author: John Pollock <pollockjj@gmail.com>
Date: Mon May 25 22:13:06 2026 -0500
Add SeedVR2 integration coverage
diff --git a/.github/workflows/test-unit.yml b/.github/workflows/test-unit.yml
index d05179cd..c52defc7 100644
--- a/.github/workflows/test-unit.yml
+++ b/.github/workflows/test-unit.yml
@@ -2,9 +2,9 @@ name: Unit Tests
on:
push:
- branches: [ main, master, release/** ]
+ branches: [ main, master, develop, release/** ]
pull_request:
- branches: [ main, master, release/** ]
+ branches: [ main, master, develop, release/** ]
jobs:
test:
@@ -12,7 +12,6 @@ jobs:
matrix:
os: [ubuntu-latest, windows-2022, macos-latest]
runs-on: ${{ matrix.os }}
- continue-on-error: true
steps:
- uses: actions/checkout@v4
:[INFO] got prompt
[INFO] Using pytorch attention in VAE
[INFO] Using pytorch attention in VAE
[INFO] VAE load device: cuda:0, offload device: cpu, dtype: torch.float16
[INFO] Requested to load VideoAutoencoderKLWrapper
[INFO] loaded completely; 478.07 MB loaded, full load: True
[ERROR] !!! Exception during processing !!! CUDA error: an illegal memory access was encountered
Search for `cudaErrorIllegalAddress' in https://docs.nvidia.com/cuda/cuda-runtime-api/group__CUDART__TYPES.html for more information.
CUDA kernel errors might be asynchronously reported at some other API call, so the stacktrace below might be incorrect.
For debugging consider passing CUDA_LAUNCH_BLOCKING=1
Compile with `TORCH_USE_CUDA_DSA` to enable device-side assertions.
[ERROR] Traceback (most recent call last):
File "G:\AI\ComfyUI\execution.py", line 536, in execute
output_data, output_ui, has_subgraph, has_pending_tasks = await get_output_data(prompt_id, unique_id, obj, input_data_all, execution_block_cb=execution_block_cb, pre_execute_cb=pre_execute_cb, v3_data=v3_data)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\execution.py", line 336, in get_output_data
return_values = await _async_map_node_over_list(prompt_id, unique_id, obj, input_data_all, obj.FUNCTION, allow_interrupt=True, execution_block_cb=execution_block_cb, pre_execute_cb=pre_execute_cb, v3_data=v3_data)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\execution.py", line 310, in _async_map_node_over_list
await process_inputs(input_dict, i)
File "G:\AI\ComfyUI\execution.py", line 298, in process_inputs
result = f(**inputs)
^^^^^^^^^^^
File "G:\AI\ComfyUI\nodes.py", line 382, in encode
t = vae.encode(pixels)
^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\comfy\sd.py", line 1301, in encode
model_management.raise_non_oom(e)
File "G:\AI\ComfyUI\comfy\model_management.py", line 394, in raise_non_oom
raise e
File "G:\AI\ComfyUI\comfy\sd.py", line 1292, in encode
out = self.first_stage_model.encode(pixels_in)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\comfy\ldm\seedvr\vae.py", line 2345, in encode
p = super().encode(x)
^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\comfy\ldm\seedvr\vae.py", line 2203, in encode
h = self.slicing_encode(x)
^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\comfy\ldm\seedvr\vae.py", line 2272, in slicing_encode
return self._encode(x)
^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\comfy\ldm\seedvr\vae.py", line 2225, in _encode
h = self.encoder(_x, memory_state=memory_state)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\venv\Lib\site-packages\torch\nn\modules\module.py", line 1776, in _wrapped_call_impl
return self._call_impl(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\venv\Lib\site-packages\torch\nn\modules\module.py", line 1787, in _call_impl
return forward_call(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\comfy\ldm\seedvr\vae.py", line 1643, in forward
sample = down_block(sample, memory_state=memory_state)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\venv\Lib\site-packages\torch\nn\modules\module.py", line 1776, in _wrapped_call_impl
return self._call_impl(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\venv\Lib\site-packages\torch\nn\modules\module.py", line 1787, in _call_impl
return forward_call(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\comfy\ldm\seedvr\vae.py", line 1299, in forward
hidden_states = resnet(hidden_states, temb=None, memory_state=memory_state)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\venv\Lib\site-packages\torch\nn\modules\module.py", line 1776, in _wrapped_call_impl
return self._call_impl(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\venv\Lib\site-packages\torch\nn\modules\module.py", line 1787, in _call_impl
return forward_call(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\comfy\ldm\seedvr\vae.py", line 1201, in forward
hidden_states = self.conv1(hidden_states, memory_state=memory_state)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\venv\Lib\site-packages\torch\nn\modules\module.py", line 1776, in _wrapped_call_impl
return self._call_impl(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\venv\Lib\site-packages\torch\nn\modules\module.py", line 1787, in _call_impl
return forward_call(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\comfy\ldm\seedvr\vae.py", line 796, in forward
return self.slicing_forward(input, memory_state)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\comfy\ldm\seedvr\vae.py", line 865, in slicing_forward
input[i] = self.memory_limit_conv(
^^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\comfy\ldm\seedvr\vae.py", line 747, in memory_limit_conv
x[idx] = torch.cat([prev_cache[idx], x[idx]], dim=split_dim - 1)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
torch.AcceleratorError: CUDA error: an illegal memory access was encountered
Search for `cudaErrorIllegalAddress' in https://docs.nvidia.com/cuda/cuda-runtime-api/group__CUDART__TYPES.html for more information.
CUDA kernel errors might be asynchronously reported at some other API call, so the stacktrace below might be incorrect.
For debugging consider passing CUDA_LAUNCH_BLOCKING=1
Compile with `TORCH_USE_CUDA_DSA` to enable device-side assertions.
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "G:\AI\ComfyUI\execution.py", line 541, in execute
comfy.model_management.reset_cast_buffers()
File "G:\AI\ComfyUI\comfy\model_management.py", line 1367, in reset_cast_buffers
synchronize()
File "G:\AI\ComfyUI\comfy\model_management.py", line 1942, in synchronize
torch.cuda.synchronize()
File "G:\AI\ComfyUI\venv\Lib\site-packages\torch\cuda\__init__.py", line 1108, in synchronize
return torch._C._cuda_synchronize()
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
torch.AcceleratorError: CUDA error: an illegal memory access was encountered
Search for `cudaErrorIllegalAddress' in https://docs.nvidia.com/cuda/cuda-runtime-api/group__CUDART__TYPES.html for more information.
CUDA kernel errors might be asynchronously reported at some other API call, so the stacktrace below might be incorrect.
For debugging consider passing CUDA_LAUNCH_BLOCKING=1
Compile with `TORCH_USE_CUDA_DSA` to enable device-side assertions.
[INFO] Prompt executed in 1.41 seconds
Fatal Python error: Aborted
Stack (most recent call first):
File "G:\AI\ComfyUI\main.py", line 371 in prompt_worker
File "C:\Users\user\AppData\Local\Programs\Python\Python311\Lib\threading.py", line 975 in run
File "C:\Users\user\AppData\Local\Programs\Python\Python311\Lib\threading.py", line 1038 in _bootstrap_inner
File "C:\Users\user\AppData\Local\Programs\Python\Python311\Lib\threading.py", line 995 in _bootstrap
Extension modules: sqlalchemy.cyextension.collections, sqlalchemy.cyextension.immutabledict, sqlalchemy.cyextension.processors, sqlalchemy.cyextension.resultproxy, sqlalchemy.cyextension.util, greenlet._greenlet, markupsafe._speedups, yaml._yaml, PIL._imaging, multidict._multidict, yarl._quoting_c, propcache._helpers_c, _brotli, aiohttp._http_writer, aiohttp._http_parser, aiohttp._websocket.mask, aiohttp._websocket.reader_c, frozenlist._frozenlist, charset_normalizer.md, numpy._core._multiarray_umath, numpy.linalg._umath_linalg, torch._C, torch._C._dynamo.autograd_compiler, torch._C._dynamo.eval_frame, torch._C._dynamo.guards, torch._C._dynamo.utils, torch._C._fft, torch._C._linalg, torch._C._nested, torch._C._nn, torch._C._sparse, torch._C._special, numpy.random._common, numpy.random.bit_generator, numpy.random._bounded_integers, numpy.random._pcg64, numpy.random._mt19937, numpy.random._generator, numpy.random._philox, numpy.random._sfc64, numpy.random.mtrand, psutil._psutil_windows, PIL._imagingft, av._core, av.logging, av.bytesource, av.buffer, av.audio.format, av.error, av.dictionary, av.container.pyio, av.option, av.descriptor, av.format, av.utils, av.stream, av.container.streams, av.sidedata.encparams, av.sidedata.motionvectors, av.sidedata.sidedata, av.opaque, av.packet, av.container.input, av.container.output, av.container.core, av.codec.context[ERROR] Error handling request from 127.0.0.1
Traceback (most recent call last):
File "G:\AI\ComfyUI\venv\Lib\site-packages\aiohttp\web_protocol.py", line 510, in _handle_request
resp = await request_handler(request)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\venv\Lib\site-packages\aiohttp\web_app.py", line 569, in _handle
return await handler(request)
^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\venv\Lib\site-packages\aiohttp\web_middlewares.py", line 117, in impl
return await handler(request)
^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\middleware\cache_middleware.py", line 27, in cache_control
response: web.Response = await handler(request)
^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\server.py", line 87, in deprecation_warning
response: web.Response = await handler(request)
^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\server.py", line 181, in origin_only_middleware
response = await handler(request)
^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\server.py", line 195, in block_external_middleware
response = await handler(request)
^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\custom_nodes\ComfyUI-MemoryVisualization\__init__.py", line 204, in aimdo_vram_status
free_cuda, total_vram = torch.cuda.mem_get_info(device)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "G:\AI\ComfyUI\venv\Lib\site-packages\torch\cuda\memory.py", line 897, in mem_get_info
return torch.cuda.cudart().cudaMemGetInfo(device)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
torch.AcceleratorError: CUDA error: an illegal memory access was encountered
Search for `cudaErrorIllegalAddress' in https://docs.nvidia.com/cuda/cuda-runtime-api/group__CUDART__TYPES.html for more information.
CUDA kernel errors might be asynchronously reported at some other API call, so the stacktrace below might be incorrect.
For debugging consider passing CUDA_LAUNCH_BLOCKING=1
Compile with `TORCH_USE_CUDA_DSA` to enable device-side assertions.
, av.video.format, av.video.reformatter, av.plane, av.video.plane, av.video.frame, av.video.stream, av.codec.hwaccel, av.codec.codec, av.frame, av.audio.layout, av.audio.plane, av.audio.frame, av.audio.stream, av.filter.link, av.filter.context, av.filter.graph, av.filter.filter, av.filter.loudnorm, av.audio.resampler, av.audio.codeccontext, av.audio.fifo, av.bitstream, av.video.codeccontext, _cyutility, scipy._cyutility, scipy._lib._ccallback_c, scipy.ndimage._nd_image, scipy.ndimage._rank_filter_1d, scipy.special._ufuncs_cxx, scipy.special._ellip_harm_2, scipy.special._special_ufuncs, scipy.special._gufuncs, scipy.special._ufuncs, scipy.special._specfun, scipy.special._comb, _ni_label, scipy.ndimage._ni_label, regex._regex, sentencepiece._sentencepiece, scipy.integrate._odepack, scipy.integrate._quadpack, scipy.integrate._vode, scipy.integrate._dop, scipy.sparse._sparsetools, _csparsetools, scipy.sparse._csparsetools, scipy.linalg._fblas, scipy.linalg._flapack, scipy.linalg.cython_lapack, scipy.linalg._cythonized_array_utils, scipy.linalg._solve_toeplitz, scipy.linalg._batched_linalg, scipy.linalg._decomp_lu_cython, scipy.linalg._matfuncs_schur_sqrtm, scipy.linalg._matfuncs_expm, scipy.linalg._linalg_pythran, scipy.linalg.cython_blas, scipy.linalg._decomp_update, scipy.sparse.linalg._dsolve._superlu, scipy.sparse.linalg._eigen.arpack._arpacklib, scipy.sparse.linalg._propack, scipy.optimize._group_columns, scipy._lib.messagestream, scipy.optimize._trlib._trlib, scipy.optimize._lbfgsb, _moduleTNC, scipy.optimize._moduleTNC, scipy.optimize._slsqplib, scipy.optimize._minpack, scipy.optimize._lsq.givens_elimination, scipy.optimize._zeros, scipy._lib._uarray._uarray, scipy.linalg._decomp_interpolative, scipy.optimize._bglu_dense, scipy.optimize._lsap, scipy.spatial._ckdtree, scipy.spatial._qhull, scipy.spatial._voronoi, scipy.spatial._hausdorff, scipy.spatial._distance_wrap, scipy.spatial.transform._rotation_cy, scipy.spatial.transform._rigid_transform_cy, scipy.optimize._direct, scipy.interpolate._fitpack, scipy.interpolate._dfitpack, scipy.interpolate._dierckx, scipy.interpolate._ppoly, scipy.interpolate._interpnd, scipy.interpolate._rbfinterp_pythran, scipy.interpolate._rgi_cython, scipy.special.cython_special, scipy.stats._stats, scipy.stats._biasedurn, scipy.stats._stats_pythran, scipy.stats._levy_stable.levyst, scipy.stats._ansari_swilk_statistics, scipy.sparse.csgraph._tools, scipy.sparse.csgraph._shortest_path, scipy.sparse.csgraph._traversal, scipy.sparse.csgraph._min_spanning_tree, scipy.sparse.csgraph._flow, scipy.sparse.csgraph._matching, scipy.sparse.csgraph._reordering, scipy.stats._sobol, scipy.stats._qmc_cy, scipy.stats._rcont.rcont, scipy.stats._qmvnt_cy, av.subtitles.stream, scipy.signal._sigtools, scipy.signal._max_len_seq_inner, scipy.signal._upfirdn_apply, scipy.signal._spline, scipy.signal._sosfilt, scipy.signal._peak_finding_utils, kiwisolver._cext, _cffi_backend, sklearn.__check_build._check_build, pyarrow.lib, pandas._libs.tslibs.ccalendar, pandas._libs.tslibs.np_datetime, pandas._libs.tslibs.dtypes, pandas._libs.tslibs.base, pandas._libs.tslibs.nattype, pandas._libs.tslibs.timezones, pandas._libs.tslibs.fields, pandas._libs.tslibs.timedeltas, pandas._libs.tslibs.tzconversion, pandas._libs.tslibs.timestamps, pandas._libs.properties, pandas._libs.tslibs.offsets, pandas._libs.tslibs.strptime, pandas._libs.tslibs.parsing, pandas._libs.tslibs.conversion, pandas._libs.tslibs.period, pandas._libs.tslibs.vectorized, pandas._libs.ops_dispatch, pandas._libs.missing, pandas._libs.hashtable, pandas._libs.algos, pandas._libs.interval, pandas._libs.lib, pyarrow._compute, pandas._libs.ops, pandas._libs.hashing, pandas._libs.arrays, pandas._libs.tslib, pandas._libs.sparse, pandas._libs.internals, pandas._libs.indexing, pandas._libs.index, pandas._libs.writers, pandas._libs.join, pandas._libs.window.aggregations, pandas._libs.window.indexers, pandas._libs.reshape, pandas._libs.groupby, pandas._libs.json, pandas._libs.parsers, pandas._libs.testing, sklearn._cyutility, sklearn.utils._isfinite, sklearn.utils.sparsefuncs_fast, sklearn.utils.murmurhash, sklearn.utils._openmp_helpers, sklearn.metrics.cluster._expected_mutual_info_fast, sklearn.metrics._dist_metrics, sklearn.metrics._pairwise_distances_reduction._datasets_pair, sklearn.utils._cython_blas, sklearn.metrics._pairwise_distances_reduction._base, sklearn.metrics._pairwise_distances_reduction._middle_term_computer, sklearn.utils._heap, sklearn.utils._sorting, sklearn.metrics._pairwise_distances_reduction._argkmin, sklearn.metrics._pairwise_distances_reduction._argkmin_classmode, sklearn.utils._vector_sentinel, sklearn.metrics._pairwise_distances_reduction._radius_neighbors, sklearn.metrics._pairwise_distances_reduction._radius_neighbors_classmode, sklearn.metrics._pairwise_fast, sklearn.preprocessing._csr_polynomial_expansion, sklearn.preprocessing._target_encoder_fast, skimage.measure._ccomp, skimage.measure._moments_cy, skimage.measure._find_contours_cy, skimage.measure._marching_cubes_lewiner_cy, numba.core.typeconv._typeconv, numba._helperlib, numba._dynfunc, numba._dispatcher, numba.core.runtime._nrt_python, _win32sysloader, win32api, numba.np.ufunc._internal, numba.experimental.jitclass._box, msgpack._cmsgpack, xxhash._xxhash, cuda.bindings._bindings.cydriver, cuda.bindings.cydriver, cuda.bindings.driver, cuda.bindings._bindings.cyruntime_ptds, cuda.bindings._bindings.cyruntime, cuda.bindings.cyruntime, cuda.bindings.runtime, srsly.ujson.ujson, srsly.msgpack._epoch, srsly.msgpack._packer, srsly.msgpack._unpacker, blis.cy, thinc.backends.cblas, cymem.cymem, preshed.maps, blis.py, thinc.backends.linalg, murmurhash.mrmr, thinc.backends.numpy_ops, thinc.layers.premap_ids, thinc.layers.sparselinear, spacy.symbols, preshed.bloom, spacy.strings, spacy.attrs, spacy.parts_of_speech, spacy.morphology, spacy.lexeme, spacy.tokens.morphanalysis, spacy.tokens.token, spacy.tokens.span, spacy.tokens.span_group, spacy.tokens._retokenize, spacy.tokens.doc, spacy.vectors, spacy.vocab, spacy.training.align, spacy.training.alignment_array, spacy.pipeline._parser_internals.nonproj, spacy.training.example, spacy.training.gold_io, spacy.matcher.levenshtein, spacy.matcher.matcher, spacy.matcher.dependencymatcher, spacy.matcher.phrasematcher, spacy.tokenizer, spacy.pipeline.pipe, spacy.pipeline.trainable_pipe, spacy.pipeline._parser_internals.stateclass, spacy.pipeline._parser_internals.transition_system, spacy.kb.kb, spacy.kb.candidate, spacy.kb.kb_in_memory, spacy.ml.parser_model, thinc.extra.search, spacy.pipeline._parser_internals._beam_utils, spacy.pipeline.transition_parser, spacy.pipeline._parser_internals.arc_eager, spacy.pipeline.dep_parser, spacy.pipeline._edit_tree_internals.edit_trees, spacy.pipeline.tagger, spacy.pipeline.morphologizer, spacy.pipeline._parser_internals.ner, spacy.pipeline.ner, spacy.pipeline.senter, spacy.pipeline.sentencizer, google._upb._message, lxml._elementpath, lxml.etree, PIL._imagingcms, scipy.io.matlab._mio_utils, scipy.io.matlab._streams, scipy.io.matlab._mio5_utils, pywt._extensions._dwt, pywt._extensions._cwt, pywt._extensions._pywt, pywt._extensions._swt (total: 338) |
Author
That looks like a VAE encode OOM. You might need to adjust the size of the VAE input tile. Try 256 or smaller and see if the error goes away, or try to reduce your upscale magnitude and try again. |
Uhm, when doing vae decode/encode, doesn't it usually fall back to tiled VAE processing if there isn't enough memory? I don't know what happened, I didn't restart my PC, but I left the pc for few hours, and run comfy again, and now it's working fine again. The 1st run used the exact same settings & input image as when the CUDA illegal memory access happened. For the 3rd run, I simply increased the multiplier in SeedVR2Resize Node, and it fell back to tiled VAE. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@comfy/ldm/seedvr/model.py`:
- Around line 891-901: The mmRoPE branch currently passes already
window-expanded txt_q/txt_k (txt_q_repeat/txt_k_repeat) into repeat_concat_idx,
causing a double-repeat and corrupting packed Q/K; modify the logic around
self.rope(mm) so that repeat_concat_idx is given the base text lengths/tensors
(or skip its internal text repeat) when txt_q_repeat/txt_k_repeat are used—i.e.,
keep the per-window expansion inside the rope.mm path and ensure
concat_win/unconcat_win (from repeat_concat_idx) are computed from non-repeated
txt inputs (or adjust repeat_concat_idx to accept a flag to disable second
repeating) so packed Q/K align with mmRoPE rotations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c65bd074-4678-47f8-8619-8142f6ab8433
📒 Files selected for processing (13)
comfy/ldm/seedvr/model.pycomfy/model_detection.pytests-unit/comfy_extras_test/test_seedvr2_conditioning.pytests-unit/comfy_extras_test/test_seedvr2_nodes.pytests-unit/comfy_extras_test/test_seedvr2_post_processing.pytests-unit/comfy_test/model_detection_test.pytests-unit/comfy_test/seedvr_vae_forward_test.pytests-unit/comfy_test/test_seedvr2_dtype.pytests-unit/comfy_test/test_seedvr2_internals.pytests-unit/comfy_test/test_seedvr2_model.pytests-unit/comfy_test/test_seedvr2_vae_decode.pytests-unit/comfy_test/test_seedvr2_vae_tiled.pytests-unit/comfy_test/test_seedvr_progressive_sampler.py
💤 Files with no reviewable changes (3)
- tests-unit/comfy_extras_test/test_seedvr2_post_processing.py
- tests-unit/comfy_test/seedvr_vae_forward_test.py
- tests-unit/comfy_test/test_seedvr2_dtype.py
🚧 Files skipped from review as they are similar to previous changes (2)
- comfy/model_detection.py
- tests-unit/comfy_test/model_detection_test.py
Comment on lines
+891
to
+901
| vid_q, vid_k, txt_q, txt_k = self.rope( | ||
| vid_q, vid_k, window_shape, txt_q_repeat, txt_k_repeat, txt_shape_repeat, cache_win | ||
| ) | ||
| else: | ||
| vid_q, vid_k = self.rope(vid_q, vid_k, window_shape, cache_win) | ||
|
|
||
| txt_len_win = cache_win("txt_len", lambda: txt_len.repeat_interleave(window_count)) | ||
| all_len_win = cache_win("all_len", lambda: vid_len_win + txt_len_win) | ||
| concat_win, unconcat_win = cache_win( | ||
| "mm_pnp", lambda: repeat_concat_idx(vid_len_win, txt_len, window_count) | ||
| ) |
There was a problem hiding this comment.
Fix mmRoPE text Q/K double-repeat before varlen packing.
In the self.rope.mm branch, txt_q/txt_k are already expanded per-window (txt_q_repeat/txt_k_repeat) and then passed into repeat_concat_idx(...), which repeats text streams again. That makes packed Q/K inconsistent with intended per-window rotations and can silently corrupt attention outputs.
A safe fix is to avoid applying a second repeat on text once the mmRoPE-expanded tensors are in play (or keep base text tensors here and let repeat_concat_idx be the only repeater).
As per coding guidelines "comfy/**: Focus on ... performance implications in hot paths" and backward-compatible correctness in core execution paths.
🤖 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 `@comfy/ldm/seedvr/model.py` around lines 891 - 901, The mmRoPE branch
currently passes already window-expanded txt_q/txt_k (txt_q_repeat/txt_k_repeat)
into repeat_concat_idx, causing a double-repeat and corrupting packed Q/K;
modify the logic around self.rope(mm) so that repeat_concat_idx is given the
base text lengths/tensors (or skip its internal text repeat) when
txt_q_repeat/txt_k_repeat are used—i.e., keep the per-window expansion inside
the rope.mm path and ensure concat_win/unconcat_win (from repeat_concat_idx) are
computed from non-repeated txt inputs (or adjust repeat_concat_idx to accept a
flag to disable second repeating) so packed Q/K align with mmRoPE rotations.
- Reduce SeedVR2 coverage down to production unit tests - Route SeedVR2 7B through Comfy varlength attention - Disable SeedVR2 RoPE cache reuse after the upstream DynamicVRAM change
pollockjj
force-pushed
the
seedvr2-native-support
branch
from
a5729f0 to
fc4a135
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Workflows
Example workflows are available here:
https://gist.github.com/pollockjj/8d40c875e9eacb9b560709faef4ea31f
Models
https://huggingface.co/pollockjj/SeedVR2
Model repo - contains all released SeedVR2 models with conditioning embedded so only one model file needed.
Tracking
Linear: https://linear.app/comfyorg/issue/CORE-6/support-seedvr2
Validation