Skip to content

ops: opt out of deferred weight init if subclassed#12967

Merged
comfyanonymous merged 1 commit intoComfy-Org:masterfrom
rattus128:prs/dynamic-vram-fixes/embedding-gguf-half-subclassing
Mar 15, 2026
Merged

ops: opt out of deferred weight init if subclassed#12967
comfyanonymous merged 1 commit intoComfy-Org:masterfrom
rattus128:prs/dynamic-vram-fixes/embedding-gguf-half-subclassing

Conversation

@rattus128
Copy link
Copy Markdown
Contributor

@rattus128 rattus128 commented Mar 15, 2026

#12949

If a subclass BYO _load_from_state_dict and doesn't call the super() the needed default init of these weights is missed and can lead to problems for uninitialized or left over meta weights.

I never reproduced this issue. This is a theoretical defensive fix based on the backtrace.

Regression Tests:
Linux 5090 WAN2.2 Q8 GGUF ✅
Linux 5090 ZiT GGUF (+TE) ✅
Windows, 5060, WAN Q8 2.2 GGUF ✅ (no commit charge surge)

If a subclass BYO _load_from_state_dict and doesnt call the super() the
needed default init of these weights is missed and can lead to problems
for uninitialized weights.
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 15, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: eab1f87e-aa89-4eb4-b894-c46482fad3ec

📥 Commits

Reviewing files that changed from the base of the PR and between 0904cc3 and 1bc133c.

📒 Files selected for processing (1)
  • comfy/ops.py

📝 Walkthrough

Walkthrough

The PR modifies guard conditions in Linear and Embedding classes within comfy/ops.py. The existing Windows and aimdo_enabled flags are supplemented with an additional check verifying that the class's _load_from_state_dict implementation matches the expected implementation from disable_weight_init. These enhanced guards apply to both the __init__ and _load_from_state_dict methods. When subclasses override _load_from_state_dict, the new condition causes the code path to bypass the custom lazy initialization mechanism.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding a guard to opt out of deferred weight initialization when a subclass overrides _load_from_state_dict.
Description check ✅ Passed The description is directly related to the changeset, explaining the defensive fix for handling subclasses with custom _load_from_state_dict implementations and includes relevant regression test results.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.

OpenGrep is compatible with Semgrep configurations. Add an opengrep.yml or semgrep.yml configuration file to your project to enable OpenGrep analysis.

@comfyanonymous comfyanonymous merged commit e84a200 into Comfy-Org:master Mar 15, 2026
13 checks passed
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