Repository navigation
[feat] Extend disaggregated MiniMax-H3 stage with VAE encoders - #6939
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
This PR appears to be related to model: MinimaxH3. Model owners: @david6666666 @asukaqaq-s, 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. |
|
this should be able to deploy across nodes @yuanwu2017 |
| additional_information.get("text_encoder_output") if isinstance(additional_information, Mapping) else None | ||
| ) | ||
| if not isinstance(payload, Mapping) or not payload: | ||
| raise OmniClientError("MiniMax H3 diffusion stage requires encoder conditioning from the encoder stage") |
There was a problem hiding this comment.
[P1] This breaks the standard single-stage deployment. recipes/MiniMaxAI/MiniMax-H3.md (unchanged by this PR) serves vllm serve MiniMaxAI/MiniMax-H3 --omni, which routes here with the model_loaded["text_encoder"] default of True and no stage-0 encoder — forward/prepare_encode now raise "requires encoder conditioning from the encoder stage" on every request; at base that path was served by the deleted load_text_encoder/encode_prompt branch. Please restore the in-pipeline fallback or retire single-stage explicitly (update MiniMax-H3.md, note the break in the PR body); the new line in MiniMax-H3-Disaggregated.md ("standard MiniMax H3 recipes remain single-stage and continue to run all components inside the diffusion pipeline") is now false.
There was a problem hiding this comment.
Fixed: the mode dispatch at pipeline_minimax_h3.py:1718-1729 restores the in-pipeline path for the default text_encoder=true, vae_encoder=true, the Qwen encoder import is back, both recipes now match the behavior, and test_minimax_h3_execution_modes.py covers all three modes. Resolved.
| audio_lengths.append(int(length)) | ||
| if audio_lengths: | ||
| if any(length < 80 or length > 600 for length in audio_lengths): | ||
| raise ValueError("MiniMax H3 audio references must each be between 2 and 15 seconds") |
There was a problem hiding this comment.
[P2] Only standalone audios get the [2,15]s OmniClientError check at prepare time (validate_reference_audio_waveforms(standalone_audios) in stage_input_processors/minimax_h3.py); embedded video audios are first caught here with a bare ValueError inside make_omni_output, and gpu_ar_model_runner has no per-request catch around it — a ref2va video whose audio stream is <2s or >15s kills the whole stage-0 engine instead of 4xx-ing that request (the old DiT-side check was OmniClientError). Smallest fix: validate the non-None video_audio_inputs in prepare_encoder_prompt too — waveform and sample rate are already in hand.
There was a problem hiding this comment.
Fixed: prepare_encoder_inputs validates the combined effective audio list (embedded video audios + standalone) at prepare time with OmniClientError (encoder_processing.py:451-457), and _encode_local_media converts any residual ValueError to OmniClientError and propagates it rank-consistently via all_gather (pipeline_minimax_h3.py:1643-1657). Resolved.
| ) -> AbstractContextManager: | ||
| parallel_size = int(getattr(self, "parallel_size", 1)) | ||
| if parallel_size <= 1: | ||
| return nullcontext() |
There was a problem hiding this comment.
[P2] The old encode_image unconditionally forced self.model.parallel_tiling = False; with parallel_size == 1 (video_vae "leader" mode — set_parallel_size is never called) this nullcontext now runs image/keyframe encode with the constructor value int(config["vae_parallel_tiling"]) (vae.py:122), so encode becomes tiled whenever the gated checkpoint config is truthy — silent drift vs the reference whole-frame keyframe encode. Can't verify the checkpoint config in-repo: either restore the forced-False for parallel_size == 1 or add a base-vs-head A/B showing tiled image encode matches reference keyframe latents.
There was a problem hiding this comment.
Fixed: encode_image forces parallel_tiling = False whenever parallel_size <= 1, with save/restore in the finally block (vae.py:430-456); the multi-rank path additionally falls back to rank-local tiling with a warning on tile shortage. Resolved.
| minimax_h3_prepare_denoise_rows, | ||
| minimax_h3_publish_denoise_progress, | ||
| ) | ||
| from .encoder import MiniMaxH3Qwen3VLEncoder |
There was a problem hiding this comment.
[P3] With this import removed, MiniMaxH3Qwen3VLEncoder (~1500 lines in diffusion/models/minimax_h3/encoder.py) has no production caller left — only tests (test_minimax_h3_contract.py, test_minimax_h3_encoder_sdpa.py, test_minimax_h3_dlo_lifecycle.py) still exercise it, so CI stays green on a dead module. Delete the module and its tests here, or note why they're kept.
There was a problem hiding this comment.
Resolved by the single-stage restore — the pipeline imports MiniMaxH3Qwen3VLEncoder again (pipeline_minimax_h3.py:99), so the module has its production caller back.
hsliuustc0106
left a comment
There was a problem hiding this comment.
Requesting changes primarily for the single-stage break: MiniMaxH3Pipeline now requires stage-0 encoder conditioning unconditionally, but recipes/MiniMaxAI/MiniMax-H3.md (unchanged) still documents vllm serve MiniMaxAI/MiniMax-H3 --omni, which was served by the deleted in-pipeline encoder — every request on the standard deployment fails, and the new MiniMax-H3-Disaggregated.md line claiming standard recipes remain single-stage is now false. Please restore the in-pipeline fallback or retire single-stage explicitly (recipe + PR body).
Also worth addressing before merge: the embedded-video-audio [2,15]s check surfaces as a bare ValueError inside stage-0 make_omni_output (engine-fatal, vs the old DiT-side OmniClientError 4xx), and encode_image no longer forces parallel_tiling = False on the single-rank path. Details in the inline comments.
Signed-off-by: Jader <yjader@foxmail.com>
Signed-off-by: Jader <yjader@foxmail.com>
Signed-off-by: Jader <yjader@foxmail.com>
Signed-off-by: Jader <yjader@foxmail.com>
Signed-off-by: Jader <yjader@foxmail.com>
7abb255 to
0e3377a
Compare
- Reject weights for disabled encoder components with a clear ValueError. - Enforce the shared 15-second budget across embedded and standalone audio. - Update regression tests for the unified audio budget contract. Signed-off-by: Jader <yjader@foxmail.com>
Signed-off-by: Jader <yjader@foxmail.com>
| # Embedded soundtracks and standalone references share one request-level | ||
| # duration budget. Validate the combined list so a mixed request cannot | ||
| # bypass the 15-second total limit by passing each category separately. | ||
| validate_reference_audio_waveforms(audio_inputs) |
There was a problem hiding this comment.
Keep embedded and standalone audio budgets separate, matching encode_media.
There was a problem hiding this comment.
This logic and corresponding tests have been fixed in aabd895. Thanks
| "latent_t": context["latent_t"], | ||
| "latent_h": context["latent_h"], | ||
| "latent_w": context["latent_w"], | ||
| "audio_t": context["audio_t"], |
There was a problem hiding this comment.
Save preencode_mp4, preencode_batch_frames, and video_codec_options in the step state.
| from vllm_omni.diffusion.models.minimax_h3.pipeline_minimax_h3 import MiniMaxH3Pipeline | ||
|
|
||
| pipeline = object.__new__(MiniMaxH3Pipeline) | ||
| pipeline.load_text_encoder = False |
There was a problem hiding this comment.
Set load_vae_encoder=False in this fixture; prepare_encode now reads it.
Signed-off-by: Jader <yjader@foxmail.com>
Signed-off-by: Jader <yjader@foxmail.com>
Signed-off-by: Jader <yjader@foxmail.com>
Signed-off-by: Jader <yjader@foxmail.com>
Re-port the MiniMax H3 timeline-guide feature onto main's split encoder / diffusion conditioning refactor (vllm-project#6939). Conflict resolution: - pipeline_minimax_h3.py: drop the request-preparation code main moved into ``encoder_processing`` (``_resolve_shape``, the old ``encode_prompt``, the ``_encode_*_conditions`` family, ``_validate_reference_image``) and re-attach the guide pipeline to ``_prepare_encoder_conditioning_inputs`` so guides work from either locally or remotely produced ``MiniMaxH3EncoderConditioning``. Reference/target row budgets are now exact instead of pre-decode estimates, and the distributed-error helpers come from ``distributed_errors`` rather than main's inline copies. - Guided requests need a local VAE encoder in the diffusion stage, so a ``vae_encoder=false`` diffusion stage now rejects guides with a 400. - encoder_processing.py: route the canvas through the shared ``resolve_minimax_h3_target_canvas`` and apply the guided down-only Ref2VA reference policy while preparing encoder images, keeping the Qwen presentation and the VAE reference in agreement. - stage_input_processors/minimax_h3.py: keep the pre-Qwen guide rejections on main's renamed ``prepare_encoder_prompt``. - tests: rebuild the timeline-guide suites on the encoder-conditioning entry point, keep the ``distributed_errors`` unit coverage, and drop cases whose subjects main deleted. Signed-off-by: shykoe <shykoe@gmail.com>
Rebase the unique vllm-project#6264 delta onto main a769d93 after merged vllm-project#6939 (d554e6b). Preserve upstream VAE encoder, multi-GPU/offload and request-start callback changes. Retain full-payload routing, TP fanout, inline cleanup and legacy text compatibility. Reconcile tests without replaying dependency commits. Signed-off-by: yuanwu <yuan.wu@intel.com>
…project#6939) Signed-off-by: asukaqaq-s <1311722138@qq.com> Signed-off-by: Jader <yjader@foxmail.com> Co-authored-by: Jader <yjader@foxmail.com> Co-authored-by: Hongsheng Liu <liuhongsheng4@huawei.com> Signed-off-by: Matthieu Laneuville <matthieu.laneuville@surf.nl>
…project#6939) Signed-off-by: asukaqaq-s <1311722138@qq.com> Signed-off-by: Jader <yjader@foxmail.com> Co-authored-by: Jader <yjader@foxmail.com> Co-authored-by: Hongsheng Liu <liuhongsheng4@huawei.com>
按 §3 表的 H3 推理契约一行补 §3.1,写明: - 机制:`prepare_encoder_inputs` 的 `reference_image_shape` 回调只有 diffusion pipeline 传, 分档时 bridge 不传,落回上游的"保持分辨率 + 32 对齐"。 - 实测数字:256x256 参考图下,单档编到 2048(视觉条件 64x64+1、latent 128x128), 分档保持 256(8x8+1、latent 16x16)。 - 性质:并行的两条设计,非我方缺陷(上游没这个回调,两条路径都走上游几何); 已登记为有意偏离。 - 分档启动时若契约几何与上游不同会打 warning,不拒绝启动(上游的两 stage 档会用)。 - 现网 11 份档全是单 stage,因此走契约几何那条路。 - "让分档也应用契约几何" 单独立项:回调要跨 stage 边界,或把 resize 挪到 stage1 —— 但 stage0 在 vllm-project#6939 之后已把参考图编成 latent,stage1 无法再 resize。 Signed-off-by: reputationly <reputationly@gmail.com> Signed-off-by: reputationly <197039020@qq.com>
Purpose
Extend the existing disaggregated MiniMax-H3 text encoder stage with video VAE and audio WVAE encoders.
tp, video VAE usespatchorleader, and audio WVAE usesleader.MiniMaxH3Encoderon top ofMiniMaxH3TextEncoderBackbone, preserving the existing Qwen3-VL preprocessing, text embeddings, token tags, and stage interfaces.Test Plan
vLLM Version:
<fill in test environment>vLLM-Omni Commit:
10ab4704Test Result
ruff check: passedruff format: 17 files unchangedsetup.
@hsliuustc0106 @Gaohan123 @yuanwu2017 @yJader