Skip to content

fix(diffusion): load serialized H3 INT8 in ComfyUI integrated mode - #42121

Merged
mickqian merged 5 commits into
sgl-project:mainfrom
Tokha233:fix/comfy-h3-int8-loader
Oct 4, 2026
Merged

mickqian merged 5 commits into
sgl-project:mainfrom
Tokha233:fix/comfy-h3-int8-loader

Conversation

@Tokha233

@Tokha233 Tokha233 commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

Loading an H3 single-file INT8 ConvRot checkpoint through the ComfyUI integrated path currently fails with Parameter ...comfy_quant not found in custom model state dict. The loader reads the H3 metadata but discards its per-layer quantization markers, so it builds ordinary floating-point Linear parameters.

Modifications

Resolve the serialized INT8 config using the existing H3 loader utilities, pass it into model construction, and filter metadata-only keys. Preserve CPU checkpoint placement for layerwise offload. Reject conflicting runtime quantization, FSDP, and unsupported serialized formats explicitly. The existing ComfyUI QKV layout remains unchanged.

Compatibility after upstream #38040

The loader fix uses the shared checkpoint quantization resolver, which now returns ConvRotInt8Config. The regression covers both comfy_kitchen and jit serialized loading with their kernel initialization mocked; this validates metadata, weight and scale loading, not JIT inference. The full Larry8 GPU timing below was measured on the earlier Kitchen-backed revision and is not a benchmark of the new automatic JIT backend.

Accuracy Tests

  • Regression writes a real small safetensors checkpoint and runs the shared loader. INT8 weights and FP32 scales must remain exactly equal, parameters frozen, and ComfyUI QKV ordering preserved.
  • The test fails on the original loader with the missing proj.comfy_quant error, and passes with this change.
  • H3 ComfyUI unit suite: 28 passed, 1 skipped, 3 xfailed.
  • Actual 1x RTX 5090 D v2, PyTorch 2.13.0+cu130: the full Larry v4 baked INT8 checkpoint loads and completes 8-step Ref2VA sampling. Fixed seed 42, 768x512, 124 frames, Euler/beta, CFG 1, identical video/audio latent hashes across three timed repetitions.

Speed Tests and Profiling

This is a loading correctness fix, not a kernel performance claim. Successful native SGLang sampling with SDPA measured 32.905 / 32.916 / 32.947 seconds after warmup; excludes condition encoders, VAE and export. The original integrated loader fails before sampling, so there is no valid speedup ratio. No experimental attention or row-splitting changes are included in this PR.

Checklist

  • Changed-file pre-commit checks pass.
  • Added a regression test with actual safetensors loading.
  • Included accuracy and runtime validation above.
  • Followed existing loader/config conventions.

Codex assisted with implementation and testing. The reported GPU runs were actually executed; no human review is implied.


CI States

Latest PR Test (Base): ✅ Run #37113105032
Latest PR Test (Extra): ❌ Run #37113104859
Latest PR Test (AMD ROCm 10): ❌ Run #37113105086

@Tokha233

Tokha233 commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

The failing gate jobs stop at Require run-ci label; the downstream finish jobs propagate that gate result. Could a maintainer authorize the appropriate diffusion CI with run-ci?

Local validation on the submitted commit: 28 passed, 1 skipped, 3 xfailed; real H3 INT8 safetensors loading and full Larry8 sampling complete on RTX 5090 D v2. Changed-file pre-commit passes. This PR fixes checkpoint loading only; it does not claim cross-framework output equivalence or a kernel speedup. Test evidence.

@niehen6174

Copy link
Copy Markdown
Collaborator

/tag-and-rerun-ci

@github-actions github-actions Bot added the run-ci CI: run the baseline test suite on this PR label Oct 2, 2026
@Tokha233

Tokha233 commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for the review and CI authorization. I inspected the rerun failures: all four NPU diffusion jobs stop during dependency installation because the triton-ascend 3.2.2 wheel URL returns HTTP 403, before tests execute (run). The Extra workflows still stop at the separate run-ci-extra label gate.

I have left the loader patch unchanged; there is no test failure pointing to it in these logs. The baseline GPU workflows remain queued.

@Tokha233

Tokha233 commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor Author

Follow-up on the newly completed main GPU run: I inspected attempt 2 of run 36918637044. This is separate from the earlier NPU wheel-download failures.

  • H3 Ref2VA on 2×H100 completed both requests, but consistency failed: SSIM 0.8031 vs threshold 0.92 (CLIP 0.9495, PSNR 28.6611, mean absolute difference 3.1042). Both requests repeat the same metrics.
  • LTX 2.3 two-stage TI2V failed its performance limit: median denoise step 305.5608 ms vs 291.3480 ms allowed.

The H3 server log explicitly records model_path=MiniMaxAI/MiniMax-H3, revision 42ed227ee7df40d41602854ae760620d6eb651fe, comfyui_mode=false, and no transformer override. At composed_pipeline_base.py:447, load_comfyui_transformer() is selected only when both ComfyUI mode and a single-file checkpoint are active. The production change in this PR is confined to that loader; this failing official-model test does not enter the modified loading branch. LTX has no INT8 H3 loader changes either.

I have kept the quality/performance thresholds unchanged. A matching base-commit run on the same CI runner would distinguish reference/runtime drift from a PR regression; I do not have a 2×H100 reproduction locally. Could the failed GPU shard be compared with the base commit or rerun on a clean runner? The earlier single-GPU loader regression and exact serialized INT8 checks remain documented above.

AMD job logs are now available too:

  • 1-GPU AMD job 110739186484: the newly added test_integrated_h3_int8_loader_preserves_quantized_weights passed (07:24:15 UTC). The later unit sweep reports 27 failed / 3477 passed. Failure families include FP8 e4m3fnuz vs e4m3fn assumptions, CUDA-specific Qwen image fusion checks on ROCm, and performance-guard tests expecting exceptions while AMD emits warnings. None of the listed failures is the changed H3 loader test.
  • 2-GPU AMD job 110739186483: JoyEcho generated invalid audio (Input contains (near) NaN/+-Inf); AAC encoding failed and the resulting MP4 lacked a moov atom. Summary: 1 failed / 12 passed / 1 skipped. This is a separate model path.

These logs do not establish a base-commit pass or fail, so I am not claiming that CI is green or that all failures are environmental. The changed loader regression is passing; a base comparison remains needed for the broader CUDA/ROCm suites.

Signed-off-by: Tokha233 <61346912+Tokha233@users.noreply.github.com>
@Tokha233

Tokha233 commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor Author

Updated this branch with upstream a74f259 / #38040 in e6344c3.

The production loader fix still uses the shared checkpoint quantization resolver. Updated the regression's renamed Kitchen import and parametrized real safetensors loading for both comfy_kitchen (12 output features) and jit (16, matching its output alignment). Kernel initialization is mocked; weight bytes, FP32 scales, frozen parameters, QKV layout and conflicting-config rejection remain checked. This is loading coverage, not a JIT inference benchmark.

Validation in an isolated checkout with Python sources verified against the local tree: 29 passed, 1 skipped, 3 xfailed (3.78 s), on the RTX 5090 D v2 test host with compatible Torch/ComfyUI installed. Changed-file pre-commit passed.

python -m pytest -q python/sglang/multimodal_gen/test/unit/test_comfyui_h3.py

The earlier full Larry8 timing is historical Kitchen-backend evidence, not a new automatic-JIT measurement. No quality/performance thresholds were changed; latest CI results are recorded below.

Latest-head NPU follow-up (e6344c3, run 37030626476): all four diffusion NPU jobs fail in Install dependencies on the same triton-ascend==3.2.2 aarch64 wheel URL, returning HTTP 403 Forbidden. Their model-test steps do not execute. The URL comes from the existing scripts/ci/npu/npu_ci_install_dependency.sh; this PR does not change that script. Restoring the artifact access or a verified same-version mirror is needed before a useful NPU rerun. The CUDA and AMD results are tracked separately below.

Latest CUDA/AMD follow-up (2026-10-03, UTC+8), still on e6344c3:

  • CUDA run 37030627159 is complete. The 5090, B200, unit, component-accuracy and remaining GPU partitions pass. In 2-GPU partition 2, native H3 Ref2VA now passes both requests; CI reports sampled-frame SSIM 1.0000 and mean absolute difference 0.0000 against its CI reference. This is the native H3 test, not an INT8-versus-BF16 business-quality measurement, and it does not establish why the previous head failed.
  • That partition remains red after retries because of two LTX performance checks: ltx_2_two_stage_t2v final median denoise 134.2356 ms versus limit 130.9680 ms, and ltx_2_3_two_stage_ti2v_2gpus 300.0596 ms versus limit 291.3480 ms. Qwen's initial performance failure passed on retry. The PR's production changes are inside the ComfyUI H3 checkpoint loader; no LTX execution/performance thresholds are changed. A matching-base run on the same runner is still needed before assigning the cause or calling these baseline/infrastructure failures.
  • AMD 1-GPU partition 0 passes both new serialized loader cases (comfy_kitchen-12 and jit-16, with kernel initialization mocked). The full unit suite reports 3515 passed / 27 failed / 92 skipped / 3 xfailed; failures are in FP8 dtype handling, Qwen fusion/LoRA and performance-validation tests, not the added loader regressions. AMD 2-GPU partition 0 reports 12 passed / 1 failed / 1 skipped; the failing JoyEcho case logs AAC input containing NaN/Inf. Other AMD partitions are still running or queued. These are not a claim of working JIT execution on AMD.

Could a maintainer help with matching-base/same-runner comparison for the remaining CUDA LTX and AMD failures, and restoring the NPU wheel access? No test thresholds, skips, or expected outputs were changed to make CI green.

AMD follow-up at 2026-10-03 01:49 UTC+8: 1-GPU partition 1 has now completed: 4 passed, 1 failed, 1 skipped. The failure is flux3_action_http: action response contains non-finite values. This is a real output-validity failure to investigate, not a timing threshold failure.

I also grouped the 27 unit failures in partition 0: 16 performance/memory-guard tests, 4 FP8 dtype failures (including two subtests), and 7 Qwen Image 2.1 LoRA/fusion tests. There is a concrete platform-policy mismatch in the guard tests: PerformanceValidator._assert_le deliberately only warns on HIP, while multiple unit tests expect an AssertionError without pinning the platform; peak-memory validation also has a CUDA-only guard. For example, the slow-load test's 6000 ms vs 4000 ms case raises in the non-HIP branch but returns in the HIP branch. I confirmed that branch behavior with an isolated source-function probe (simulated platform flag, not an AMD GPU run). FP8 logs show float8_e4m3fn versus float8_e4m3fnuz; some Qwen fusion tests encounter the exactness fallback and do not reach the expected gate state.

The validator, the relevant guard/Qwen tests, Flux3 model and JoyEcho pipeline are byte-identical to the merged upstream base a74f259; this PR does not edit them. This source comparison does not replace a same-runner baseline for numerical/performance failures. No skips, tolerances, or output sanitization have been added.

The AMD 2-GPU partition 1 is now complete as well. Its server suite reports 11 passed, 2 failed, 1 skipped, 1 error:

  • Native H3 T2VA fails at the existing _run_full_loop platform check: MiniMax H3 full-loop denoise requires CPU, CUDA, MPS, XPU, or Ascend NPU. That allowlist does not include HIP.
  • Native H3 Ref2VA fails before generation with HTTP 400 MiniMax H3 media material is invalid, which wraps the ffprobe stage; the exposed job log does not identify its underlying exception.
  • LTX-2.5 setup fails with Hugging Face GatedRepoError / HTTP 403 for the audio VAE files. This is distinct from the CUDA LTX timing failures.
  • A subsequent standalone test_ar_models.py fails collection with LinearBase circular import: linear -> quantization.__init__ -> auto_round -> linear.

The H3 denoise/material validation and these circular-import source files are also byte-identical to a74f259. The added serialized INT8 loader unit tests passing on AMD does not mean the full native H3 pipeline passes on AMD. One AMD partition is still running and one remains queued at this refresh.

@Tokha233

Tokha233 commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor Author

I reproduced and submitted two independent fixes for concrete failures in this CI run, keeping the approved H3 loader patch unchanged:

  • fix(diffusion): preserve lazy quantization exports and per-backend lookup #42249 fixes the cold-import cycle (linear -> quantization -> auto_round -> linear). On upstream 89f2167, the new regression file gives 3 failures / 14 passes; the candidate plus existing import, ConvRot and transformer-quant tests gives 116 passes and 20 passing subtests on the RTX 5090 D v2 test host.
  • test(diffusion): cover CUDA and HIP latency guard policies #42250 fixes the platform setup in performance-guard unit tests. Running the same three files with a simulated HIP platform flag reproduces 16 failures / 70 passes on upstream; the candidate gives 90 passes with that flag and 90 passes on the native CUDA host. This is Python-policy coverage, not AMD kernel validation. Production thresholds and validator behavior are unchanged.

Both were tested against isolated Python source trees checked against upstream hashes, with stale source files removed before final revalidation. Relevant pre-commit checks pass. Could a maintainer authorize run-ci for these two PRs and verify them on AMD?

The remaining failures need separate actions: the CUDA LTX timing checks still need same-runner comparison against a74f259; AMD numerical failures require locating the first non-finite/mismatching stage; native H3's HIP eligibility and media-probe failures need platform validation; the LTX-2.5 gated files and NPU Triton wheel require CI artifact access fixes. These two PRs do not claim to resolve those issues. AMD 1-GPU partition 2 has since passed all 9 selected cases; the final partition is still running at this refresh.

Further concrete fixes submitted (2026-10-03, UTC+8):

Final revalidation used three independent upstream f6fcda8 trees, with all 4,124 / 4,125 / 4,124 Python files hash-checked against the submitted source; zero mismatches or extra Python files. All changed-file pre-commit checks pass. These are RTX 5090 D v2 / PyTorch 2.13.0+cu130 results. FNUZ arithmetic-contract coverage and HIP dispatch flags are simulated, not native MI300 execution. The real AMD full-pipeline NaN and Qwen numerical failures still need retesting; no output sanitization, threshold increases or whole-model skips were added.

The two earlier PRs' lint checks have passed. New GPU gates report Missing required label 'run-ci'. Please authorize CI for these independent fixes, especially native AMD. AMD 1-GPU partition 3 has also completed successfully, so the original AMD run is now fully finished. The approved #42121 patch remains unchanged pending upstream integration and platform validation.

All three new PRs have now passed GitHub lint as well. Native GPU jobs remain gated on run-ci; no new human reviews yet.

@Tokha233

Tokha233 commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

Synced this branch with upstream ce617d4, including the merged contiguous-layout residual dispatch fix #42240 and native FP8 fix #42255. The PR diff remains limited to the H3 serialized INT8 loader and its regression file; production/performance thresholds are unchanged.

#42240's upstream investigation specifically identifies dispatch overhead affecting ltx_2_two_stage_t2v and ltx_2_3_two_stage_ti2v_2gpus, the two latency failures seen here. Its 2×H200 measurements are not a rerun of our H100 shard, so CI still needs to establish whether those failures are resolved after this sync.

Validation on the submitted source (c0a1570), isolated checkout with all 4,126 Python files hash-checked against the commit: 29 passed, 1 skipped, 3 xfailed in the loader regression file (3.76 s pytest; 15.74 s whole process), on RTX 5090 D v2 / PyTorch 2.13.0+cu130. Changed-file pre-commit passes. This is loader coverage; a new H100 LTX end-to-end pass is not claimed.

Also submitted #42359 for a separate AMD unit-timeout contributor: batching performance-guard scenarios reduces 63 pytest startups to 7, retaining all nine scenarios over seven attempts and per-case JUnit failure assertions. Same-host full-file runtime: 888.28 s -> 117.36 s; native AMD rerun remains needed. The test change is not bundled into this loader PR.

Please evaluate the updated CUDA/AMD checks with these merged upstream fixes; remaining unmerged platform fixes are tracked in #42250, #42256 and #42257.

@Tokha233

Tokha233 commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

Additional evidence for the native H3 Ref2VA SSIM failure, from four completed CUDA jobs:

Job PR / tested merge Runner Python Minimum SSIM (both requests)
110916604659 #42121 / b3ab3bd9b h100-novita6-gpu-45 3.10.12 1.0000
111157440611 #42121 / c4765c81c h100-novita2-gpu-23 3.12.3 0.8031
111174783660 #42121 / 484a890169 h100-novita2-gpu-67 3.12.3 0.8031
111184302904 #42249 / 1831cb29a7 h100-novita9-gpu-45 3.10.12 1.0000

I downloaded failure artifact 11270663335 and recomputed pixel metrics against the pinned dbb70135 PNGs. First/middle/last SSIM reproduce as 0.863342 / 0.853237 / 0.803101. The two failed requests' three PNGs are byte-identical. This is a reproducible output difference, not merely the rounded aggregate: bedding, pose and window contours differ. CLIP/PSNR/MAE pass, but all three frames fail SSIM. The Sep-6, Sep-13 and Sep-29 reference versions do not restore the threshold either.

The parsed H3 server_args dictionaries are identical in the current failure and both passing logs above. This uses the official pinned model, TP2, no LoRA/quantization and comfyui_mode=false; it does not select the modified ComfyUI single-file loader. Between 1831cb29a7 and 484a890169, the H3 model/stage code, test case and test_utils.py are unchanged. Torch 2.13.0+cu130, Triton 3.7.1, PyAV 15.1.0, NumPy 1.26.4 and scikit-image 0.25.2 match; Python, SciPy and CUDA Python bindings differ, and the runner/source revisions also differ.

This correlation is not proof of a Python-version cause or a controlled base/head comparison. The next useful check is base 7be5e347 versus head 315e7add on the same H100 runner image, recording the system FFmpeg version plus hashes of decoded reference RGB/PCM, conditioning tensors and initial noise before comparing denoise stages. That separates input/runtime drift from a PR regression. No thresholds or GT pins have been changed; I have no local dual-H100 reproduction.

@niehen6174

Copy link
Copy Markdown
Collaborator

/rerun-failed-ci

@mickqian
mickqian merged commit fbce0d9 into sgl-project:main Oct 4, 2026
216 of 243 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

diffusion SGLang Diffusion run-ci CI: run the baseline test suite on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants