Skip to content

refactor(vllm-omni/hi3): reuse FusedMultimodalCondition.concat for DiT capture padding - #569

Open
chi030303 wants to merge 2 commits into
Tencent-Hunyuan:mainfrom
chi030303:refactor/hi3-fused-capture-concat
Open

chi030303 wants to merge 2 commits into
Tencent-Hunyuan:mainfrom
chi030303:refactor/hi3-fused-capture-concat

Conversation

@chi030303

Copy link
Copy Markdown
Contributor

Summary

hi3_fused_conditions right-padded per-request fused_mm_capture dicts with its own ragged-L helpers, and that copy still filled position_ids with 0. Those ids are upstream HunyuanImage3's KV-cache write slots, so a constant 0 pad writes every pad position of a short row onto cache slot 0.

HunyuanImage3FusedMultimodalCondition.concat already pads position_ids with each row's continuing indices (#479). This deletes the adapter's _pad_to / _pad_attn_mask assembly and calls that concat. hi3_ar_fused_conditions is unchanged.

Related Issue

Fixes #525

Test Plan

CPU check of hi3_fused_conditions against the previous padding, on three ragged captures (L = 7 / 11 / 9, 2 rows each) and on two equal-length captures:

  • input_ids, attention_mask, gen_image_mask, and gen_timestep_scatter_index match the previous 0-pad assembly
  • the short row's position_ids pad is [7, 8, 9, 10] instead of [0, 0, 0, 0]; the length-9 row's pad is [9, 10]
  • equal lengths leave position_ids as 0..4
  • a missing fused_mm_capture still raises the existing RuntimeError

Not run: pre-commit; reason: the hook runner is not installed in this environment.
Not run: GPU rollout or diffusion replay; reason: train-side replay calls predict_noise without a KV cache, so this position_ids change has no numeric effect on the current path. The zero pad becomes a real KV overwrite only on a cached forward.

Compatibility / Risk

No config, checkpoint, or API change. Ragged DiT captures now carry continuing position_ids instead of a 0 pad. Callers still receive {"fused": HunyuanImage3FusedMultimodalCondition}.

Reviewer Notes

The deleted helpers had no other callers. Hi3ImageOutputAdapter and Hi3DitRecaptionOutputAdapter only pass the returned dict through. An independent read of the diff found no blocking issues.

AI-assisted implementation.

Checklist

  • I reviewed the changed code and removed unrelated/generated artifacts.
  • I updated tests, docs, and configs where needed, or explained why not.

Made with Cursor

The adapter's local ragged-L padding still zero-filled position_ids.
Those ids are the KV-cache write slots, so delegate padding to
HunyuanImage3FusedMultimodalCondition.concat.
@github-actions github-actions Bot added the need review Ready and waiting for review label Oct 5, 2026
The docstring hook rejects anything longer than a single line.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

need review Ready and waiting for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(vllm-omni/hi3): reuse FusedMultimodalCondition.concat for DiT capture padding (drops position_ids zero-pad)

1 participant