{{ message }}
Repository navigation
Studio: refuse a single-file diffusion load that would exhaust host RAM instead of crashing - #14510
Open
danielhanchen wants to merge 11 commits into
Open
danielhanchen wants to merge 11 commits into
danielhanchen wants to merge 11 commits into
Conversation
An fp8 or fp8_scaled ComfyUI Krea 2 file (13 GB) on a GPU with no fp8 runtime is widened to bf16 in host RAM (about 26 GB) before the denoiser is placed. With too little free memory the process was killed (Linux OOM killer, Windows commit limit), taking the whole app down instead of failing the load. Price the new host memory from the safetensors header (tensors already at the compute dtype stay memory-mapped and cost nothing) and refuse with a readable message before any widening, in both the original-layout loader and the ComfyUI-quantized loader. UNSLOTH_DIFFUSION_HOST_RAM_CHECK=0 skips the check.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
The account paying for this security review has reached its Codex usage limits. The payer can check the Codex usage dashboard. For personal accounts, using credits requires enabling “Use credits for security reviews” in Code review settings. If you do not manage the paying account, contact this repository's admins. |
This was referenced Oct 21, 2026
Open
1 of 5 tasks
for more information, see https://pre-commit.ci
With an fp16 pipeline and _keep_in_fp32_modules the ComfyUI loader only postpones the dtype choice; with no fp8 runtime (never available off bf16) every quantized layer is still expanded. Price those layers as widened, and block formats under a key map, which are always dequantized.
An nvfp4 header describes packed uint8 with K / 2 columns, while the dequantization allocates the dense N x K weight, so the check counted half of it. nvfp4 is the only packed ComfyUI format the loader reads; int8, fp8 and mxfp8 store one code per element.
The header estimate could not see per-layer choices the loader makes later, so it kept drifting from what was really written: layers a runtime filter rejects are still dequantized, a kept nvfp4 weight gets a nibble-swapped copy plus untiled scales, and FNUZ float8 tensors were skipped entirely. A HostRamBudget now holds free RAM minus the small-host reserve for the whole load. The header estimate still refuses up front (it now prices FNUZ float8 too), and every allocating helper (dequant, dequant_block, the nibble swap, scale untiling, dtype casts, segment joins) charges the budget before it allocates, so the load is refused before the allocation that would not fit instead of running the host out of memory.
With UNSLOTH_DIFFUSION_HOST_RAM_CHECK=0 (or host RAM unreadable) the budget could never refuse, but it was still installed and charged by every helper. Leave it inactive so the kill switch costs nothing. [skip ci]
This was referenced Oct 21, 2026
The in-load check summed every allocation it was told about and never released any, so it over-counted copies freed again before the next layer (the nibble-swapped codes and untiled scales of an nvfp4 fallback load, a false refusal of a load that fits) and still missed copies made where nothing charged (the row reorder torch.cat of a key-mapped layer such as MiniMax-H3's fc1, the tiled scale buffer of a runtime Linear). Keep the header estimate as the first gate, then before each allocation refuse only if it plus the small-host reserve exceeds the host's live available memory (host_ram_mib, cgroup capped). Freed buffers come back in the next reading and unchecked copies show up in it. The key map join and tile_scales now check their own allocation too. The kill switch still installs no check, and the error text is unchanged. [skip ci]
Arm the in-load check by the widened estimate as well as the file size: a 900 MiB nvfp4 file widens to 3.6 GiB of bf16, so a small file is exempt only when its estimate is small too. Dequantize fallback nvfp4 / mxfp8 layers in bounded row chunks straight into the output and check the output plus one chunk of int32 / float32 intermediates before starting, instead of the output alone. Every op is per row, so the result is bit-identical to one pass. Check every full-size host copy the loaders make before it is made: row reorders of kept codes and their scales rebuilt by a stock converter, the joined codes and scales of a runtime block Linear, the HunyuanImage row join, and contiguous copies of non-contiguous kept codes.
…8 dequant peak A HostRamShortfallError raised during the narrow-tag conversion was caught by the retry handler and the conversion ran again at full float64 width, where a converter's joins allocate more before the next check. Re-raise it. That is the only broad except in the single-file loaders that wraps a host RAM check. int8 / fp8 dequantization holds float32 codes and their float32 product together, and ConvRot's batched matmul a float32 copy and result on top, so check 8 bytes per element (12 with ConvRot) before starting instead of the output size alone. [skip ci]
Member
Author
|
@codex review |
1 similar comment
Member
Author
This branch has not been deployed
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
Loading a custom Krea 2 safetensors from the custom models folder (the ComfyUI
diffusion_modelsfile, usually fp8 or fp8_scaled, 13 GB) could take the whole app down (#14474). Studio already supports these files: the family is detected from the header, the DiT comes from the file and the text encoder / VAE fromkrea/Krea-2-Turbo. What kills the process is host RAM:load_original_layout_transformer, which widens every fp8 tensor to the compute dtype in host RAM (value.to(want)). That is about 23 to 26 GB of new memory for the 13B Krea 2 DiT.load_comfy_quant_transformer. On a GPU with no fp8 runtime (anything below sm_89, such as the reporter's RTX 3060 Ti), every layer is dequantized to bf16 the same way.This PR prices the new host memory from the safetensors header before any widening. Tensors already at the compute dtype stay memory-mapped and cost nothing. When the widened weights plus the small-host reserve do not fit in free RAM, the load is refused with a readable error. This applies to both loaders and to every family that uses them:
UNSLOTH_DIFFUSION_HOST_RAM_CHECK=0(the existing small-host switch) skips the check, and loads under 1 GiB of widening are never checked.Evidence
scale_weight/scaled_fp8,model.diffusion_model.prefix, diffusers layout) all load throughDiffusionBackend.load_pipelinefrom a custom folder onmain. The load path itself is fine.main: the process is SIGKILLed (exit 137) with no Python error.mainvs this branch:tests/test_diffusion_single_file_host_ram.py. Four tests fail onmainand pass here: the fp8 file is refused before any weight is read, the fp8_scaled file with no fp8 runtime is refused, a bf16 file is never refused, and the widened byte count matches. Two pass on both: the same files load when there is room, and the kill switch / unreadable RAM never refuse.test_diffusion_comfy_quant.py,test_diffusion_krea2.py,test_diffusion_single_file_krea2_hunyuanimage.py,test_diffusion_single_file_registry.py,test_diffusion_small_host.py,test_hub_safetensors_single_file.py,test_video_comfy_quant.py. No change between base and head.Review follow-ups
The header estimate alone kept missing per-layer choices the loader makes later (layers a runtime filter rejects, fp16 deferred dequantization, the packed NVFP4 width, FNUZ float8, nibble-swapped NVFP4 copies, key-map row reorders, tiled scale buffers). So the header estimate is now only the first gate, refusing before any read. During the load, every helper that allocates host memory (
_dequant,dequant_block,swap_nibbles,untile_scales,tile_scales, the dtype casts in both loaders, the segment join, the key-maptorch.cat) checks its allocation against the host's live available memory (cgroup-capped, about 0.5 ms per reading) minus the reserve, and the load is refused before the allocation that would not fit. Because it reads live memory rather than keeping a running total, freed buffers count as free again and a copy made where nothing checks still shows up in the next reading. Files under 1 GiB are not checked, andUNSLOTH_DIFFUSION_HOST_RAM_CHECK=0installs no check at all.Real-size check: a 13.2 GiB Krea 2 fp8_scaled ComfyUI file (rebuilt from the bf16 transformer) loading at bf16 on CPU, with a simulated 64 GiB host whose free memory falls as the process's anonymous memory grows (measured peak 23.26 GiB, reserve 6.4 GiB):
Each review item has a test that fails before its fix and passes after, including a 48-layer NVFP4 fallback load that fits and was refused by the earlier running-total version.
Not covered