Repository navigation
[Model] MiniMax-H3: overlap Ref2VA CPU noise preparation - #8577
Conversation
Signed-off-by: GUOGUO <xwdong1998@163.com>
|
Author self-review completed using the repository's precheck-pr, full mode, against main
Ready for maintainer review. Publication-baseline CPU validation (main @vllm-omni-review-bot Please review this PR. |
|
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: @david6666666 @fhfuih @Bounty-hunter Routing: @david6666666 via module of the changed files, semantic router, model owner; @fhfuih via module of the changed files, CODEOWNERS; @Bounty-hunter via module of the changed files @Dong1017, 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 routing recordAssigned Strict on cursor (cursor-grok-4.6-high) under experiment |
Omni ReviewBot triage noteAutomated triage of commit
These are automated triage suggestions only — the final decision belongs to the maintainers. |
|
@vllm-omni-review-bot Please review this PR. @david6666666 Could you review this change as the MiniMax-H3 model owner, especially its model semantics, correctness evidence and performance attribution? Scope: CPU initial/visual-noise preparation overlapped with encoding, exact preparation-input checks, and a resident-VAE YAML using existing options. The five changes have separate scopes: #8129, #8229, #8577, #8578 and #8579. The shared eight-rank Ref2VA evidence and per-change attribution are summarized in the Issue #8228 update. The existing Draft/Ready status is retained. |
|
Reviewed head [P2] Update the mixed-reference test's VAE mock — The existing No production correctness defect identified. Reported CI, Python syntax checks, and diff hygiene pass. Pytest and NPU benchmarks were not rerun because an isolated sandbox for the fork could not start; the finding above is based on source tracing and an independent standard-library Mock behavior check. |
Signed-off-by: GUOGUO <xwdong1998@163.com>
|
@hsliuustc0106 Fixed in 6c2fdea. The existing VAE mock now returns Both |
| init_latents=init_latents, | ||
| refine=refine, | ||
| precomputed_initial_noise=precomputed_initial_noise, | ||
| precomputed_visual_condition_noise=precomputed_visual_condition_noise, |
There was a problem hiding this comment.
These parameters keep both CPU noise buffers alive throughout denoising, even after _build_denoise_inputs() has moved the inputs to the device. context.pop() only removes the caller's references. Could we release the consumed buffers here to avoid retaining them on every rank?
There was a problem hiding this comment.
Fixed in beb6c38.
Deleting the callee's parameters alone was insufficient: the caller's temporary keyword dictionary retained the CPU tensors until diffuse returned. diffuse now consumes the precomputed noise directly from the existing request context while building denoise inputs, so neither the context nor the caller's keyword dictionary retains the consumed buffers throughout denoising. The step-execution path still builds its inputs directly.
A regression test uses real CPU noise, actual denoise-input construction with CPU-to-meta transfers, and weak references to assert that every consumed host noise tensor is released before selecting the DiT transformer. The existing bitwise denoise-input equivalence tests remain enabled. 107 targeted CPU tests passed, and all applicable pre-commit checks passed. This fix addresses buffer ownership without introducing a new performance claim.
| else nullcontext() | ||
| ) | ||
| # Independent CPU generators preserve sampling; join the worker on request exit. | ||
| with executor_context as executor: |
There was a problem hiding this comment.
If text or media encoding fails, this with still waits for the noise worker and runs any queued visual-noise job. Could we cancel pending futures on failure and add an encoder-failure regression test? The running task can still be joined for cleanup.
There was a problem hiding this comment.
Fixed in beb6c38.
The request-owned executor now uses shutdown(wait=True, cancel_futures=True) in finally. A text- or media-encoder exception cancels pending initial/visual-noise jobs while still joining any running work. The original exception propagates to the caller.
Two regression cases inject failures at the text and media encoder boundaries, hold a real executor worker, and check that both pending noise jobs are cancelled and the running worker has finished. Together with the noise equivalence and lifetime cases, 107 targeted CPU tests passed. All applicable pre-commit hooks passed; the previous head's reported checks were successful, and the follow-up needs its own CI results.
Signed-off-by: GUOGUO <xwdong1998@163.com>
Signed-off-by: GUOGUO <xwdong1998@163.com>
|
@david6666666 Fixed the PR-owned NVIDIA/AMD CPU failures in 2e77dfd295. Both failures were The test now consumes those buffers at the mocked Matched CPU reproduction on refreshed head
Coverage includes execution modes, initial/visual noise preparation and lifetime, packing, and H3 stage-input processing. Tests used the existing isolated CPU harness ( Latest failed checks on the previous head are now separated by cause:
NPU 8670 passed on |
Signed-off-by: GUOGUO <xwdong1998@163.com>
|
The remaining NVIDIA failure in build 16968 was a worker-RPC timeout, before either accuracy assertion could run. Both 50-step requests were still advancing when the 600-second executor budget expired: I2VA reached step 36/50 at approximately 18 s/step; Ref2VA reached step 23/50 at approximately 28 s/step. The guard covers the whole Fix: Validation: all applicable pre-commit and commit checks passed; importing the real executor with the test environment confirmed a 3600-second worker budget; the existing timeout-related CPU tests passed (2 passed). The full NVIDIA 50-step accuracy rerun remains the final check. On the preceding head @david6666666 Can you add ready label again when you have a chance? Thanks |
|
CI check for head
The remaining failure is AMD 13583, Simple · Other Test: All assertions about the expected domain-lock timeout report passed. The failure is at This is outside the H3 CPU-preparation path. The test directly exercises Host Weight Runtime; the test file is byte-identical to current main (blob |
Accept ordered timeline guides as multi_modal_data["timeline_guides"]: each entry has a strict integer frame_index (negative counts from the aligned output end) and an image or a video clip, optionally plus audio. - timeline_guides.py: native 17k+5 clip cadence, pixel-frame placement and per-guide audio window; guides are center-cropped to the output canvas with the existing H3 image/audio loaders and a 24 FPS ffmpeg decode. - conditioning/encoder_processing: guides travel in MiniMaxH3EncoderMediaInput, so the existing patch-parallel VAE broadcast covers them, and encode_media emits guide blocks plus anchor rows. Guided Ref2VA references are scaled down to the output area. - pipeline: guides pack before references for every task, FL2VA keyframes become trailing image guides, and the guided noise recipe is selected per request. Guided Ref2VA requests still reuse the background target noise from vllm-project#8577 but skip its dependent visual-condition precompute, since the guided recipe draws per condition over guide + reference rows. Guided requests are rejected with cache acceleration, FastH3, LoRA/Turbo, distilled schedules, continuation, latent refine/upscale, latent-mask editing, lock_source audio, and a separate encoder stage. Signed-off-by: shykoe <shykoe@gmail.com>
新增 §3.2。要点: - 关的是生产端(不再起工作线程、不再写 request_context);diffuse / _build_denoise_inputs 一侧的 precomputed_* 形参与分支按上游保留,减少同步冲突。 - 为什么必须关:diffuse 先判 precomputed_initial_noise,会短路 MiniMaxH3RequestNoisePlan —— 后者正是 §5.3 逐位一致的依据;预计算用的是上游的 self._initial_noise,另一个生成器。 - 为什么以前没暴露:**合并时两边拼出来的**。上游 74ba8ab 有生产端、没有那行 rebind; origin/main 与 10-04 有 rebind、没有生产端;合并后两者都在,写入先进局部 {}、 随后 context 被 _prepare_encoder_conditioning_inputs 的返回值替换,于是成了死写。 死写是偶然的,而这个偶然正好挡住了 §5.3 回归。请求路径与 step 路径都是死写。 - 下次同步怎么查:git grep 那两个标识符 + 两条会变红的用例。 - 明确写"不要顺手修 rebind"——那会引入 §5.3 回归。 - 重新开放的条件:预计算改成走 noise_plan(移动绘制时机,不是搬运行),独立优化。 Signed-off-by: reputationly <reputationly@gmail.com> Signed-off-by: reputationly <197039020@qq.com>
先更正上一提交的判断:这条死写**是合并时解冲突产生的**,不是我方特性。 我在 7fd06a7 里只对比了 origin/main 就下了"我方特性"的结论,是我漏对照上游。 四版本同一文件三项计数(precompute / request_context=context / 那行 rebind): origin/main 0 / 0 / 有 merge/upstream-20261004 0 / 0 / 有 74ba8ab(上游) 13 / 2 / 没有 HEAD 13 / 2 / 有 预计算来自上游 681ae74(vllm-project#8577 "overlap Ref2VA CPU noise preparation"): 上游有生产端、**没有**那行 rebind,所以在上游预计算真正生效;是我方 `context = self._prepare_encoder_conditioning_inputs(...)` 与上游的生产端拼在一起, 写入才落进被丢弃的 {} 里 —— 两处都是死写(请求路径 :4741、step 路径 :4914 都取同一个被替换后的 context)。挡住 §5.3 回归的是这个偶然,不是我们的意图。 一、关掉生产端(按"只关生产端") - 不再计算 initial_noise_params / visual_noise_params、不再起 ThreadPoolExecutor、 不再 submit 两个噪声任务、不再写 request_context、不再做预测形状校验。 - `diffuse` 与 `_build_denoise_inputs` 的 `precomputed_*` 形参与分支**原样保留**, `header = [(text_conditioning is not None, initial_noise_params, visual_noise_params)]` 一行与上游逐字节一致,后续同步冲突面最小。 - 生产端原址留一段注释:为什么关、上游为何没有这个问题、以及**重新开放的条件 是把预计算改成走 noise_plan(移动绘制时机,不是搬运行)**。 - 顺带删掉因此变为未使用的 `ThreadPoolExecutor` 与 `minimax_h3_imgvid_cond_noise_rows` 导入。 二、修正 warning 的触发条件(a5a1c43a4 的遗留) 原判据是 `not self.load_text_encoder`,但 resize 实际发生在 `_prepare_local_conditioning`, 其门槛是 `load_text_encoder or load_vae_encoder`(:4233)。故按 vae_encoder 拆成两种说法: - vae_encoder 也不在本地:整套几何走上游,契约的 reference_image_* 全部不生效; - vae_encoder 在本地:视觉条件在本地按契约几何编码,而**文本条件**是 stage0 用上游几何算出来的 —— 同一个请求的两半对参考图不一致。这一句是新增的信息。 三、测试 - 上游的 `test_encoder_failure_cancels_pending_noise_and_joins_worker` 钉的正是我关掉的生产端 (它断言 submit 出 2 个 future,并 monkeypatch `module.ThreadPoolExecutor` —— 那个名字已不存在)。改写为 `test_encoder_failure_leaves_no_noise_worker_and_an_untouched_context`: 编码器失败仍要抛出、不留工作线程、且 context 里没有 precomputed_*。生产端复活即变红。 - `test_minimax_h3_execution_modes.py` 里在 `forward` 之后、pop 之前读取 `diffuse` 收到的 request_context,断言其中没有 precomputed_* —— 真实调用边界上的钉子。 - 生产端关掉后,`test_ref2va_initial_noise_preparation.py` 其余 10 条(含全部消费侧) 与 execution_modes 全部通过。 手册同步新增 §3.2 登记为"下次同步必查"偏离。 Signed-off-by: reputationly <reputationly@gmail.com> Signed-off-by: reputationly <197039020@qq.com>
…t#8577) Signed-off-by: GUOGUO <xwdong1998@163.com>
上游同步(2026-10-12 起步)的最终合入。合并后: - 代码与已验证的 sync/upstream-20261012 分支逐字节相同(git diff --stat 为空) - 保住 main 侧全部 13 个文档提交,含手册 §3.1(ref2va 几何仅单档)与 §3.2(已关掉上游 vllm-project#8577 生产端,下次同步必查) - 真机验收见 docs/sync-20261012/:§13 出包与补测、§14 §6.3 头条复核 - 单测 1627 passed / 0 failed(-m "not (H100 or B200)",那 9 个是 H100/B200 专用且需 HF 公网) H3 三条必须在下次同步复核的偏离已写进手册 §3.1 / §3.2。 已独立复核并由构造法确认 §5.3 种子契约未受影响(§13.2)。 Signed-off-by: reputationly <reputationly@gmail.com>
Purpose
Move Ref2VA CPU random-noise preparation off the serial preparation path while preserving the existing random draws. A request-owned single-worker executor prepares initial video/audio noise and visual-reference noise during text/media encoding. The first output consumes the prepared tensors; continuation, external conditioning and subsequent outputs retain their existing behavior.
Visual-condition mixing keeps NPU input on NPU and preserves CPU FP32 coefficient rounding and operation order. Video-shape prediction reuses the checkpoint processor's actual temporal/spatial alignment methods and shared padding metadata; the pipeline checks predicted dimensions against encoded dimensions before consuming the prepared noise.
Related to #8228. This PR contains three H3 production files, two CPU test files and a reproducible eight-rank resident-VAE YAML. The YAML uses existing configuration options: TP8 text encoder, USP8 DiT, patch8/tile VAE, offload
dit,text_encoder, and resident video/audio VAE. It includes the measured rank-local text-encoder weight transfer. Allocator telemetry and KV distribution are separate PRs; #8129 and #8229 contain the media optimizations.Test Plan
vLLM Version: 0.29.0 for device measurements; 0.30.0 for CPU regression.
vLLM-Omni Commit: device measurements use
0e6791e9c4925c3539352cff2c2738bd48a1a65aplus recorded candidate changes. The publication branch is based on maind5a3380103df2fc827f095c60dbfb6e1c5655fd3.Hardware/workload: 8×Ascend 910B1, 64 GB/device; PyTorch 2.10.0+cpu, torch-npu 2.10.0.post4. Fixed 15-second reference video with audio, seed 2101, requested 50 inference steps, flow shifts 12/3, short edge 768, aspect ratio 16:9, fps 24. Output: 1344×768, 362 frames with audio.
pytest -q tests/diffusion/models/minimax_h3/test_ref2va_initial_noise_preparation.py \ tests/diffusion/models/minimax_h3/test_minimax_h3_packing.py vllm serve "$MODEL" --omni \ --config recipes/MiniMaxAI/minimax-h3-ref2va-8-rank-resident.yamlThe device experiment ran equivalent CLI parameters. The new YAML parses successfully; it does not add a new residency mechanism.
Test Result
Complete-combination acceptance
Each service completed a fixed-input 50-step warmup on all eight ranks before three alternating unprofiled measurement pairs. Combining this PR, #8578, #8579, #8129, #8229 and resident VAE configuration gives client E2E medians 2253.664743 → 2239.304066 s, saving 14.360677 s (0.637%). Each pair improves: 15.435865 / 18.683454 / 14.360677 s.
A separate profiled pair covers all eight device event databases per arm. Common time with no device task on any rank falls 6.631556 → 3.039535 s (−54.17%); the preparation window falls 3.670694 → 0.941927 s (−74.34%). These are complete-combination results, with the CPU preparation mechanisms identified above; they are not assigned wholly to this PR.
Publication-baseline CPU validation (main
d5a3380103plus the three scoped candidates, vLLM 0.30.0 / PyTorch 2.13.0+cu130, accelerator devices hidden): 160 passed, 1 deselected, 17 warnings in 129.78 s, normal process exit. This covers the 23 preparation tests, 18 allocator/lifecycle/revert tests, and 119 KV cases. Tests run in isolation from the full repository plugin fixture, which requires vLLM 0.31. The one deselected existing example-CLI test imports that 0.31 engine interface and fails in the installed 0.30 runtime; its error is retained separately. Required cache-dit and pytest-mock packages are installed in task-local directories; the shared environment is unchanged.