fix(fp8): honor every declared skip list, and stop overshooting the RAM reservation - #9538
Open
Pfannkuchensack wants to merge 2 commits into
Open
fix(fp8): honor every declared skip list, and stop overshooting the RAM reservation#9538Pfannkuchensack wants to merge 2 commits into
Pfannkuchensack wants to merge 2 commits into
Conversation
…AM reservation Review follow-ups for invoke-ai#9414 and invoke-ai#9415. Depends on invoke-ai#9415: the docs below describe Anima's FP8 support, which lands there. Read `_keep_in_fp32_modules` alongside `_skip_layerwise_casting_patterns`. Diffusers' `enable_layerwise_casting()` unions both; we replaced that call with our own hook-based path and were reading only the first, so a model declaring the second would lose its exclusions silently. Verified to protect nothing extra today - on Krea-2, Wan 14B, Z-Image and FLUX.1 - so this changes nothing now and stops being a trap later. Release the state dict before the FP8 cast in the Z-Image and Krea-2 single-file loaders. `load_state_dict(..., assign=True)` aliases every param to its `sd` tensor, so the compute-dtype originals stayed reachable while `param.data.to(float8)` allocated the fp8 copies, putting peak RAM ~50% over what `make_room()` reserved (~17.4GB actual against ~11.5GB reserved for Z-Image). Nothing reads `sd` after the load. Add `test_z_image_fp8_wiring.py`. Deleting the cast call from the Z-Image single-file loader previously left the whole model_manager suite green. The new tests fail on that, on removing `sd.clear()`, and on the aliasing premise itself, should torch ever stop assigning by reference. Use `get_model_compute_dtype()` in the Z-Image denoise loop instead of `transformer.dtype`. It is correct today only because `x_pad_token` happens to be parameter zero and is never cast; move the pad tokens under a submodule and the loop starts feeding float8 into `F.linear`. Reword the comment above the Z-Image cast. Dropping `.scale_weight` / `scaled_fp8` is not "filtering out metadata" - for a ComfyUI scaled-fp8 checkpoint it loads unscaled weights. That bug is pre-existing and out of scope here, but the comment read as though the cast made it safe. Update the FP8 docs, which still said Z-Image was excluded for a dtype mismatch and listed it in the troubleshooting exclusion list - the opposite of what the code has done since invoke-ai#9414. Add Anima and its LLLite adapters, and document that a model's own declared exclusions are honored on top of the generic skip list, with the measured cost: Wan 14B gives up ~221 MiB of savings, Krea-2 ~38 MiB, Anima ~18 MiB, FLUX.1 and Qwen-Image nothing.
Pfannkuchensack
requested review from
JPPhoto,
blessedcoolant,
dunkeroni and
lstein
as code owners
August 25, 2026 00:48
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
Six review follow-ups, none of which changes what FP8 storage does on a correctly-behaving model today. Two are real defects, three are guards and correctness hardening, one is user-facing documentation that currently says the opposite of what the code does.
1.
_keep_in_fp32_moduleswas not honored. Diffusers'enable_layerwise_casting()unions two class attributes before casting —_skip_layerwise_casting_patternsand_keep_in_fp32_modules. We replaced that call with our own hook-based path in #9231 and only ever read the first, so a model declaring the second would have lost its exclusions silently. Both are now read through_model_declared_skip_patterns().This is inert today, and I checked rather than assumed — on Krea-2, Wan 14B, Z-Image and FLUX.1 it protects zero additional modules. Wan's
time_embeddersits undercondition_embedder, which its_skip_layerwise_casting_patternsalready names;scale_shift_tableis a bareParameter, not a castable layer; Krea-2's entries are allnorm*, already covered by_FP8_DEFAULT_SKIP_PATTERNS. So: no behaviour change now, and it stops being a trap for the next architecture we add.2. Peak RAM overshot the
make_room()reservation.load_state_dict(sd, assign=True)aliases every parameter to its state-dict tensor, so the dict keeps the whole model alive a second time. The FP8 cast then allocated each fp8 copy while the compute-dtype original was still reachable throughsd, putting peak RAM roughly 50% over what was reserved — about 17.4 GB actual against an ~11.5 GB reservation for Z-Image.sd.clear()before the cast lets each original free as soon as its parameter is cast. Nothing readssdafter the load, in either loader. Same shape existed in the Krea-2 single-file loader; fixed there too.3. The single-file FP8 wiring had no regression guard. Deleting the
_apply_fp8_layerwise_castingcall from the Z-Image single-file loader left the entiretests/backend/model_managersuite green — the dead toggle #9414 fixed could come straight back with CI passing.test_z_image_fp8_wiring.pycloses that, and also pins thesd.clear()fix and the aliasing premise it rests on. (#9415 gets the equivalent guard for Anima.)4.
transformer.dtypein the Z-Image denoise loop was dormant, not safe.z_image_denoise.pybuiltlatent_model_inputfromtransformer.dtype, which reports the float8 storage dtype once FP8 storage is on. It happens to work only becauseget_parameter_dtypereturns the first floating-point parameter innamed_parameters()order, which is the root-levelx_pad_token— not a Linear, so never cast. Move the pad tokens under a submodule, or put a Linear ahead of them, and the loop starts feeding float8 intoF.linear. Both sites now useget_model_compute_dtype(), which is whatbackend/util/fp8.pyexists for, and the coupling to diffusers' parameter ordering is gone.5. A comment blessed a pre-existing bug. The comment above the Z-Image cast described dropping
.scale_weight/scaled_fp8as filtering out metadata. It isn't: for a ComfyUI scaled-fp8 checkpoint those keys are the scales, and discarding them loads the raw fp8 codes unscaled — i.e. wrong weights. That bug is pre-existing and out of scope here, but the comment read as though the cast made it safe. Reworded to say plainly what happens.6. The FP8 docs contradicted the code.
fp8-storage.mdxstill carried the row| Z-Image (any variant) | No — dtype mismatch with skipped layers |and named Z-Image in the troubleshooting exclusion list a user is told to check when VRAM doesn't drop — the exact exclusion #9414 deleted. Both corrected, Anima and its LLLite adapters added, and the skip-list description now says that a model's own declared exclusions are honored on top of the generic list.That last point has a measurable cost worth documenting, since it was understated in #9414. Measured on meta-device builds with real configs, counting only what the declared lists protect beyond the generic defaults:
condition_embedder,patch_embedding)time_embed)t_embedder,x_embedder,final_layer)Wan users on a tight budget will see ~220 MiB more usage than the generic defaults alone would give. It is the right direction — it is what diffusers intends — but it should be written down.
One correction to #9415 while I was here: that PR's comment describes
x_embedder+final_layeras "~2MB of margin", which is right, but the total Anima delta is 17.8 MiB, becauset_embedderalone is 16.8 M parameters. That reconciles exactly with the size table in #9415 (2012.0 − 1994.2 MB = 17.8 MB).Related Issues / Discussions
extra_skip_patterns).Noted, deliberately not fixed here:
WanCheckpointModel._load_from_singlefilenever calls_apply_fp8_layerwise_casting, so the FP8 toggle is rendered and inert for single-file Wan checkpoints — the same dead-toggle shape #9414 and #9415 fixed for Z-Image and Anima, but on a 14B model where wiring it is clearly worth more than hiding it. Out of scope for a review follow-up; happy to open a separate PR.Also unchanged: the ComfyUI scaled-fp8 key filtering in the Z-Image loader (point 5). Same issue raised on #9478.
QA Instructions
No CUDA GPU needed for the automated checks; the peak-RAM check needs a real Z-Image single-file checkpoint.
Unit tests
Expect
1192 passed, 147 skipped, 1 xfailed.The new guards are load-bearing — verify they bite. Each of these must fail:
sd.clear()fromz_image.pytest_state_dict_is_released_before_the_fp8_cast_apply_fp8_layerwise_casting(...)line fromz_image.pytest_single_file_loader_applies_fp8_layerwise_castingandtest_state_dict_is_released_before_the_fp8_cast_keep_in_fp32_modulesfrom the loop in_model_declared_skip_patternstest_model_declared_skip_patterns_unions_both_diffusers_attributes,..._tolerates_missing_and_odd_declarations,test_keep_in_fp32_modules_are_not_castPeak RAM (point 2). Needs a Z-Image single-file checkpoint and FP8 Storage enabled on it. Watch the InvokeAI process's RSS across the load — on
mainit peaks around 17.4 GB against the ~11.5 GB the loader reserved; here it should track the reservation. The log line is unchanged:Regression (points 1 and 4). Generate on Z-Image with FP8 Storage on and off, fixed seed. Output must be unchanged from before this PR in both cases — points 1 and 4 are hardening, not behaviour changes. Disable the invocation cache first (
PUT /api/v1/app/invocation_cache/disable), or the second run just replays the first. Worth one FLUX.1 and one Wan generation too, since_model_declared_skip_patternsis on the shared path: FLUX.1 should be bit-identical, Wan unchanged (its_keep_in_fp32_modulesadds nothing, as measured above).Docs. Render
docs/src/content/docs/configuration/fp8-storage.mdxand check the "What FP8 Storage applies to" section — no Z-Image exclusion row, no Z-Image in the troubleshooting list, and the two tables format correctly.Merge Plan
Merge after #9415. The Anima rows in
fp8-storage.mdxdocument support that lands there; merging this first would ship docs ahead of the code. There is no textual conflict between the two branches — #9415 touches only its three Anima files, and the tests added here are appended at the end oftest_load_default_fp8.py.#9416and#9478stack above #9415 and both touch_apply_fp8_to_nn_module. This PR touches its caller and its docstring, not its signature, so it should pass through cleanly — but update the stack downward as usual rather than merging each branch againstmainseparately.No DB schema, no redux slice, no API schema change.
Checklist
fp8-storage.mdx; this is point 6 aboveWhat's Newcopy (if doing a release after this PR) — n/a, no user-visible feature change