Skip to content

[Model] Reduce MiniMax-H3 Ref2VA RGB frame conversion - #8129

Merged
david6666666 merged 5 commits into
vllm-project:mainfrom
Dong1017:fix/minimax-h3-ref2va-frame-conversion
Oct 9, 2026
Merged

david6666666 merged 5 commits into
vllm-project:mainfrom
Dong1017:fix/minimax-h3-ref2va-frame-conversion

Conversation

@Dong1017

@Dong1017 Dong1017 commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

MiniMax-H3 Ref2VA converts every decoded RGB frame through PIL before stacking, creating hundreds of temporary host objects for a 15-second reference. Copy the decoded uint8 [T, H, W, 3] array once into an owned, contiguous tensor. This reduces isolated CPU frame-conversion time from 1.339 s to 0.297 s (77.8%), with exactly equal tensors.

The effective change is three production lines plus a focused test. Other frame formats retain the existing conversion path. Reference-audio scheduling is a separate change in #8229; VAE residency is deployment configuration. Related performance tracker: #8228.

Correctness and validation

  • python -m pytest -q tests/diffusion/models/minimax_h3/test_ref2va_frame_conversion.py tests/diffusion/models/minimax_h3/test_minimax_h3_contract.py: 228 passed, 2 skipped. The test covers exact pixel values, reversed-row/noncontiguous RGB arrays, contiguous output, and independent ownership of the output storage. Applicable pre-commit hooks passed.
  • Three alternating CPU pairs on an H200 host, using the actual prepared 355-frame, 768×1344 RGB array: tensors matched elementwise in every pair.
  • Four paired full requests on 4×H200: a 15-second Ref2VA workload, 512×512 output, 24 fps, 2 diffusion steps, seed 42, encoder TP2 and DiT USP4 with DLO. All four pairs of raw video and audio arrays were elementwise identical; output shapes were [362, 512, 512, 3] and [1, 2, 482400].
  • These frame-only checks used commit 1208d320 against base 9c1656c6. The current head c04166e3 has an identical tree to the tested frame-only commit. Pytest used vLLM 0.29.0; full requests and CPU timing used vLLM 0.30.0.

Performance and eight-rank attribution

Frame-conversion scope Baseline This implementation Reduction
Isolated H200-host CPU median, three alternating pairs 1.339 s 0.297 s 1.042 s / 77.8%
Eight-device NPU capture, host function span 4.318 s 0.441 s 3.877 s / 89.8%

The NPU capture uses 8×Ascend 910B1 (64 GiB/device), source 9678c60a, a fixed 15-second reference video, seed 2101, 1344×768 output, 362 frames at 24 fps with audio, 50 executed DiT steps, DiT USP8, text TP8, and VAE tile/patch8. Both arms enable synchronized component timing and detailed CPU/NPU collection. The candidate includes this PR, #8229, VAE residency, and explicit rank-local text weight transfer.

All eight ranks' collectives were matched. Rank0 prepares the reference while the other ranks have entered the first condition broadcast; the frame-conversion span lies on this request-critical preparation path. The combination reduces first-broadcast arrival spread from 24.433 s to 18.625 s, common eight-rank time without any device task from 5.723 s to 3.448 s, and instrumented E2E from 2254.917 s to 2247.298 s. These broader results belong to the combination; the table above identifies this PR's frame-conversion scope.

@hsliuustc0106 hsliuustc0106 added diffusion codes related to diffusion models enhancement New feature or request labels Sep 24, 2026
@hsliuustc0106

Copy link
Copy Markdown
Collaborator

This PR touches tests/diffusion/, vllm_omni/model_executor/ (2 files). Based on CODEOWNERS coverage of the changed files, the most-related reviewers appear to be:

@tzhouam @gcanlin @Sy0307

Could one of you take a look when you get a chance? Thanks!

@Dong1017 Dong1017 changed the title [Model] Avoid per-frame PIL conversion for MiniMax-H3 reference videos [Model] Reduce MiniMax-H3 Ref2VA host media preparation Sep 28, 2026
…deo decode"

This reverts commit 2f819cc.

Signed-off-by: GUOGUO <xwdong1998@163.com>
@Dong1017
Dong1017 force-pushed the fix/minimax-h3-ref2va-frame-conversion branch from 2e92ac6 to c04166e Compare September 29, 2026 01:21
@Dong1017 Dong1017 changed the title [Model] Reduce MiniMax-H3 Ref2VA host media preparation [Model] Reduce MiniMax-H3 Ref2VA RGB frame conversion Oct 2, 2026
@Dong1017
Dong1017 marked this pull request as ready for review October 3, 2026 03:19
@vllm-omni-review-bot

Copy link
Copy Markdown

This PR appears to belong to: docs/design/module/model_integration.md, docs/design/module/ar_runtime.md.

Module owners: @tzhouam @fake0fan @Gaohan123

Routing: @tzhouam via module of the changed files, CODEOWNERS; @fake0fan via module of the changed files; @Gaohan123 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.

@vllm-omni-review-bot

Copy link
Copy Markdown
Omni ReviewBot routing record

Assigned Strict on zcode (GLM-5.3-Flash) under experiment fleet-strict-cursor-grok46-zcode-glm53flash-5050-c5-z10-20261002.

@Dong1017

Dong1017 commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

Author self-review: checked the effective frame-only diff, the decoded RGB array contract, exact values, contiguous output, and independent storage ownership. The current tree matches the tested frame-only commit: 228 tests passed, 2 skipped, three real-input conversion pairs matched exactly, and four H200 full-request pairs produced identical raw video/audio arrays. CPU conversion median improves from 1.339 s to 0.297 s; the PR description separates this direct result from the combined eight-device diagnosis. All current CI checks pass.

@vllm-omni-review-bot Please review this PR.

@Dong1017

Dong1017 commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

@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: RGB frame-to-tensor conversion, with exact tensor/output equivalence checks.

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.

@david6666666 david6666666 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@david6666666 david6666666 added ready label to trigger buildkite CI nightly-test label to trigger buildkite nightly test CI labels Oct 9, 2026
@david6666666

Copy link
Copy Markdown
Collaborator

please fix ci

@vllm-omni-review-bot

vllm-omni-review-bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Omni ReviewBot: superseded

The CI failure noted on dce78432a4c6 refers to an earlier head; the pull request now points at e111b9c96dcc.

Dong1017 commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor Author

@david6666666 I checked the failed CI on the current head dce78432a4c6c7240b54b303788b63f333703a80.

Check Failure and attribution Existing fix / tracker
NVIDIA build 16924 The Simple Diffusion job reports 2 failed, 7709 passed, 48 skipped. Its VAE parallel test setup accesses world_group.cpu_group, which the test fixture does not provide. The merged #8591 repairs the patch/batch VAE fixtures.
AMD build 13514 Exactly two diffusion shards failed: test_patch_parallel_decode_splits_tiles_across_ranks and test_batch_decode_matches_native_on_all_ranks. Both fail with missing cpu_group on their test world-group objects. Same merged fix: #8591.
NPU build 8643 The inspected A2 layer-test job exits while importing pytest's tests.helpers.fixtures.clean plugin: the import chain is clean plugin → pipeline registry → AuK → unchanged Qwen2.5-Omni code → missing vLLM MultiModalProcessingResult API. No test runs in that job. Directly matches the open bug #8507. The dependency/API mismatch needs to be resolved in the shared CI environment/code.

I also checked the actual files at this PR head: the batch fixture still declares only device_group, and the patch fixture still constructs SimpleNamespace(device_group=...). They do not yet include #8591's cpu_group addition. These fixtures, the distributed VAE executor, and the Qwen2.5 import are unchanged by this PR's two-file RGB-conversion diff.

For NVIDIA/AMD, the existing fixture fix can be reused by updating this branch from main and rerunning CI; there is no need to duplicate it in the RGB-conversion implementation. The NPU issue remains separately tracked by #8507. That issue has a comment suggesting a main refresh resolves an older checkout, but the current failure follows an indirect registry → AuK → Qwen import chain, and current main still imports this API in Qwen2.5. I therefore do not treat that comment as proof that the NPU failure is fixed.

Pre-commit, DCO, both wheel builds, Intel CI, and docs pass on this head. This is CI failure attribution, not a claim that all checks are green. The FP8 RMS, JoyImage, and LAPACK failures discussed for #8104 are not failures in #8129's inspected NVIDIA/AMD builds.

@david6666666 david6666666 added ready label to trigger buildkite CI nightly-test label to trigger buildkite nightly test CI and removed ready label to trigger buildkite CI nightly-test label to trigger buildkite nightly test CI labels Oct 9, 2026
@david6666666
david6666666 merged commit 88a35c0 into vllm-project:main Oct 9, 2026
6 of 9 checks passed
@Dong1017
Dong1017 deleted the fix/minimax-h3-ref2va-frame-conversion branch October 9, 2026 07:47
khairulkabir1661 pushed a commit to khairulkabir1661/vllm-omni that referenced this pull request Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

diffusion codes related to diffusion models enhancement New feature or request nightly-test label to trigger buildkite nightly test CI ready label to trigger buildkite CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants