Repository navigation
[Bugfix][MiniMax-H3] Fix Hopper keyframe and modulation precision - #7913
Conversation
Signed-off-by: david6666666 <530634352@qq.com>
|
This PR appears to belong to: docs/design/module/diffusion/offloader.md, docs/design/module/diffusion/diffusion_model_integration.md, docs/design/module/diffusion/index.md. Module owners: @Bounty-hunter @fhfuih @wtomin Routing: @Bounty-hunter via module of the changed files, CODEOWNERS; @fhfuih via module of the changed files, CODEOWNERS; @wtomin via module of the changed files, CODEOWNERS @david6666666, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer. Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment. |
Omni ReviewBot triage noteAutomated triage of commit
These are automated triage suggestions only — the final decision belongs to the maintainers. |
The keyframe TF32-only change still fails H100 I2VA accuracy. Remove the additional Hopper-only BF16 norm/affine rounding introduced by vllm-project#7693 while retaining the BF16 gated-residual boundary. This restores the existing fused FP32 contract on Hopper and leaves other architectures unchanged. Add exact tests for both kernels at a BF16 halfway value: the premature cast drops the RMS epsilon and changes 1.2265625 to 1.234375. The old BF16 branch fails both tests; the restored FP32 path passes. Signed-off-by: david6666666 <530634352@qq.com>
c8f165b to
66c500a
Compare
|
Author self-review for
AI assistance is disclosed in the PR description. Other pending pipeline jobs are not being represented as passed. |
…lm-project#7913) Signed-off-by: david6666666 <530634352@qq.com> Signed-off-by: Matthieu Laneuville <matthieu.laneuville@surf.nl>
…lm-project#7913) Signed-off-by: david6666666 <530634352@qq.com>
Purpose
Address the remaining MiniMax-H3 I2VA reference-accuracy failure on Hopper without relaxing the existing SSIM/PSNR thresholds or changing the reference assets.
Main build 15686, at
9f5521351e4471d6538e74ca66cb63cde23b9140, fails I2VA on H100 with SSIM 0.947851 / PSNR 27.785708 dB (required ≥0.97 / ≥34 dB). Ref2VA passes. The keyframe cuDNN context is still unchanged in the tested latest main,ae3880f0e39df4ba7376481ae78d9269fc42e09c.The tested base's green Build 15722 did not run the H3 I2VA/Ref2VA accuracy job (nightly upload was skipped). It is not evidence that this particular test now passes. The clean main local reproduction below is on B300, not H100.
Main advanced during validation to
1b87115c9ede2d29702c847b6da98f9e854fa593(#7449). Its only changes are ComfyUI app integration and its feature test.vllm_omni/, the accuracy/diffusion tests, test helpers, CI configuration, and dependencies are unchanged from the tested baseae3880f0e. The recorded runs remain pinned to that base and this PR head rather than claiming a separate full run on the newer commit.Diagnosis and scope
5d3e6dc1/ [Bugfix][MiniMax-H3] Use Hopper-safe modulation precision #7693 changes the fused DiT modulation precision, but leaves keyframe VAE convolution TF32 enabled. It does not address this separate precision-sensitive path, and introduces additional BF16 rounding in the fused RMSNorm/AdaLN calculation on Hopper.encode_image()explicitly uses FP32 tensors, butcudnn.flags(deterministic=True, allow_tf32=True)still permits reduced-precision TF32 convolution. Determinism and FP32 accuracy are independent controls. The resulting image latent becomes a conditioning anchor throughout denoising.cb439f3e8439, not this PR or H100.Fix
encode_image()path used by FL2VA keyframes and Ref2VA still-image references; video-reference encoding and decoding are unchanged.Extend the CPU contract test for SM86, SM90, SM100, SM103, unavailable capability, both incoming TF32 settings, exceptional exits, and the CPU no-op path. Replace the Hopper approximate/dispatch checks with exact rounding-boundary tests for both modulation kernels, runnable on any CUDA GPU. No CI YAML change is needed: the existing H3 accuracy dependency already includes
vae.pyandindexed_modulation.py.Test Plan
vLLM Version: 0.29.0
vLLM-Omni Commit: base
ae3880f0e39df4ba7376481ae78d9269fc42e09cLocal environment: 4×B300 for full accuracy; Python 3.12.3, torch 2.13.0+cu130, cuDNN 92000, Triton 3.7.1, transformers 5.14.1, diffusers 0.40.0. The local editable distribution has stale 0.27 metadata and emits a version warning; source imports were verified against the test worktree. MiniMax-H3 snapshot:
42ed227ee7df40d41602854ae760620d6eb651fe.L1 regression and H3 CPU suite, from the repository root:
Existing L4 Accuracy job / H100 reproduction (requires four GPUs, ffmpeg/ffprobe, and cached model weights or Hugging Face access):
This retains the original seed 0, 50 inference steps, 4-way Ulysses/HSDP/text-encoder TP/VAE tiling, static compilation, fixture asset SHA256 checks, and accuracy thresholds. The B300 main baseline calls the two original test functions directly with an external artifact directory; the PR run uses the pytest command above with artifacts redirected outside the checkout. Neither is an H100 run. These are single full-fixture acceptance runs, not performance measurements.
Test Result
git diff --check: passed.66c500a71) B300, original pytest command: 2 passed, 15 warnings in 717.89s. I2VA 0.974952 / 35.883204 dB; Ref2VA 0.980576 / 42.644924 dB. Both thresholds pass; metrics match the main acceptance run and the first revision (a413d0ceb, 2 passed in 713.82s). Each generated MP4 is byte-identical to its main-baseline counterpart (SHA256 I2VAffe44e473b6f3bccd78311b0a1df9042167c314de25251120cbd651198986274, Ref2VA74debfc131da7a190766a04579393fdae59ea3c55f65a87129c569959d798de3). No speedup or repeated full-video A/B claim is made.a413d0cebin Build 15724, oninf-4x8h100-3: SSIM 0.969770 / PSNR 33.947107 dB, just below both unchanged thresholds. Ref2VA passes at 0.980572 / 42.650962 dB. Disabling keyframe TF32 alone is not a complete fix.66c500a71, H100 Build 15725, again oninf-4x8h100-3: 2 passed, 14 warnings in 1293.17s; job exit 0. I2VA SSIM 0.976254 / PSNR 36.295646 dB; Ref2VA SSIM 0.980588 / PSNR 42.655355 dB. Both original accuracy thresholds pass. This is one full-fixture H100 acceptance run, not a repeated multi-node determinism claim. No local Hopper device is available, and the existing video/audio Ref2VA fixture does not establish still-image Ref2VA output quality.Pre-submit full self-review: four directly related files; no threshold/reference changes, new public API, dependencies, broad catches, hot-path copies, or new examples. The obsolete precision branch and its selector are removed rather than left unreachable. The target H100 accuracy and FP8 quality jobs now pass. Both commits have DCO sign-off; GitHub DCO, pre-commit, Python 3.11/3.12 build, NPU CI, and docs checks have passed. The broader CUDA/AMD pipelines still have pending work as of this update; no performance claim is made.
AI assistance: Codex investigated CI/code history, implemented the scoped fix and tests, ran local validation, and drafted this description.