Repository navigation
[REFACT]Refactor diffusion outputs to payload metadata - #4922
hsliuustc0106 merged 17 commits into
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. |
Signed-off-by: bjf-frz <frz123db@gmail.com>
1a7bf50 to
a950d44
Compare
|
Please fix pre-commit before review. The current failure is |
|
Continuing on the proposal/design review: the payload/metadata direction makes sense, but I think two migration issues need to be addressed before full review.
Also please fill the Test Plan/Test Result with the exact commands and coverage for formatter normalization, Cosmos3 action-only/action+video/transfer, text outputs, serving video/chat, and plugin migration behavior. |
| def _get_diffusion_extra_output_params( | ||
| self, | ||
| custom_output: dict[str, Any] | None, | ||
| output: Any, |
There was a problem hiding this comment.
avoid any datatype, check for all changes
There was a problem hiding this comment.
avoid any datatype, check for all changes
Addressed. I tightened the diffusion output protocol typing instead of using broad Any/object at the payload/metadata boundary. The new aliases in output_metadata.py define concrete payload, metadata, envelope, and raw postprocess output types. Remaining dynamic types are kept only at external object boundaries such as engine result objects.
| return out or None | ||
|
|
||
| @staticmethod | ||
| def _get_diffusion_text_output(output: Any) -> str: |
There was a problem hiding this comment.
why we only add thise function for text output?
|
Can we add a central metadata checklist/validator for the new diffusion output envelope? Right now the PR defines Suggested checks:
I would validate known canonical keys without rejecting all unknown model-specific metadata immediately. The important part is preventing typos/shape drift for fields that serving code already consumes, and testing that |
|
One more requirement for the metadata checklist: please avoid For canonical metadata fields, the checklist should name concrete value types, for example |
a950d44 to
89fa4d2
Compare
Signed-off-by: bjf-frz <frz123db@gmail.com>
89fa4d2 to
579d337
Compare
Signed-off-by: bjf-frz <frz123db@gmail.com>
Signed-off-by: bjf-frz <frz123db@gmail.com>
Signed-off-by: bjf-frz <frz123db@gmail.com>
Fixed. |
Addressed.
|
…yload-metadata-refactor Signed-off-by: bjf-frz <frz123db@gmail.com>
Signed-off-by: bjf-frz <frz123db@gmail.com>
Signed-off-by: bjf-frz <frz123db@gmail.com>
Signed-off-by: bjf-frz <frz123db@gmail.com>
| # SenseNova-U1 / BAGEL single-stage img2text / text2text), wrap it | ||
| # as a text-type response instead of an image. | ||
| is_text_output = isinstance(output_data, str) and postprocess_output.custom_output.get("text_output") is not None | ||
| is_text_output = "text" in postprocess_output.metadata |
There was a problem hiding this comment.
This uses text metadata as the output-type signal, but several image paths now attach text metadata for reasoning/COT (think_text, ar_generated_text) while the payload is still image. Those outputs will be formatted as final_output_type="text" with no images. Please key this off the text payload/primary payload instead.
| "payload": {"image": img}, | ||
| "metadata": metadata, | ||
| }, | ||
| trajectory_latents=trajectory_latents_stacked, |
There was a problem hiding this comment.
when will we del these trajectory inputs?
There was a problem hiding this comment.
when will we del these trajectory inputs?
trajectory_* can be moved into the payload envelope as the canonical path, but we should keep the existing DiffusionOutput.trajectory_* fields as a temporary backward-compatible fallback. New pipelines should write payload["trajectory"], while the formatter can still read legacy trajectory_* fields until downstream custom pipelines/tests are migrated.
| @@ -0,0 +1,229 @@ | |||
| # SPDX-License-Identifier: Apache-2.0 | |||
There was a problem hiding this comment.
add the functions for this file and all other new files from line 3
There was a problem hiding this comment.
add the functions for this file and all other new files from line 3
done
Signed-off-by: bjf-frz <frz123db@gmail.com>
Signed-off-by: bjf-frz <frz123db@gmail.com>
|
fix ci, cc @SamitHuang @wtomin verl-omni ci breaks |
Signed-off-by: bjf-frz <frz123db@gmail.com>
…yload-metadata-refactor Signed-off-by: bjf-frz <frz123db@gmail.com>
c71a7a6 to
caf8e2b
Compare
When a diffusion pipeline includes supplementary text in output metadata (e.g. HunyuanImage-3.0 CoT), the is_text_output check incorrectly flags the output as text-only, discarding generated images. Skip metadata-based text detection when primary payload key is image, video, or audio. Regression from 1a4a042 (vllm-project#4922). Signed-off-by: zzh <943967662@qq.com>
…4922) Signed-off-by: bjf-frz <frz123db@gmail.com> Signed-off-by: y_null <y_null@qq.com>
Two main-side refactors broke SANA-WM serving after the rebase: * vllm-project#4922 removed ``DiffusionOutput.custom_output``; pipeline-specific payloads now travel in the ``{"payload": ..., "metadata": ...}`` envelope that ``normalize_diffusion_postprocess_output`` splits. Passing ``custom_output=`` raised ``TypeError`` on every request. Move the Stage-1 and refiner diagnostics into a ``sana_wm`` metadata group and read the Stage-1 latents back out of the envelope before handing them to the in-process refiner. * vllm-project#5147 moved ``create_transformer_from_config`` out of ``pipeline_ltx2`` into ``ltx2_components``. Because the two-stage pipeline builds its refiner in ``__init__``, the stale import path raised ``ImportError`` at server startup, before any request arrived. Verified on H20 (96 GB) against the converted diffusers checkpoint: server starts (65.7 GiB model load), ``POST /v1/videos/sync`` returns 200 with a valid MP4 for both a 9-frame/2-step smoke request (23.7 s) and the production 161-frame/60-step/CFG-5.0 request (236.4 s, 161 frames @ 1280x704, 16 fps, 83.3 GB peak). Signed-off-by: BruceLoveDecimal <liuqihao970610@gmail.com>
Two main-side refactors broke SANA-WM serving after the rebase: * vllm-project#4922 removed ``DiffusionOutput.custom_output``; pipeline-specific payloads now travel in the ``{"payload": ..., "metadata": ...}`` envelope that ``normalize_diffusion_postprocess_output`` splits. Passing ``custom_output=`` raised ``TypeError`` on every request. Move the Stage-1 and refiner diagnostics into a ``sana_wm`` metadata group and read the Stage-1 latents back out of the envelope before handing them to the in-process refiner. * vllm-project#5147 moved ``create_transformer_from_config`` out of ``pipeline_ltx2`` into ``ltx2_components``. Because the two-stage pipeline builds its refiner in ``__init__``, the stale import path raised ``ImportError`` at server startup, before any request arrived. Verified on H20 (96 GB) against the converted diffusers checkpoint: server starts (65.7 GiB model load), ``POST /v1/videos/sync`` returns 200 with a valid MP4 for both a 9-frame/2-step smoke request (23.7 s) and the production 161-frame/60-step/CFG-5.0 request (236.4 s, 161 frames @ 1280x704, 16 fps, 83.3 GB peak). Signed-off-by: BruceLoveDecimal <liuqihao970610@gmail.com>
…4922) Signed-off-by: bjf-frz <frz123db@gmail.com>
…4922) Signed-off-by: bjf-frz <frz123db@gmail.com>
PLEASE FILL IN THE PR DESCRIPTION HERE.
Purpose
This PR refactors diffusion model outputs from the previous output + custom_output split into a unified payload + metadata envelope.
The goal is to make diffusion outputs have one clear protocol:
Previously, DiffusionOutput had two separate output channels:
The meaning was not strict.
output usually carried the main generated result:
custom_output carried extra fields that did not fit into output.
In practice, it included several different kinds of data:
For transfer paths:
For serving hints:
For some text-like paths:
So the old model was effectively:
That caused ambiguity because custom_output was not one concept.
Concrete Old Cosmos3 Examples
For Cosmos3 action-only output, the old shape was conceptually:
The problem is that this one dict mixed:
For Cosmos3 action + video output, the old shape was conceptually:
Here video was in output, but action was in custom_output, even though both are generated payloads.
For Cosmos3 transfer output, the old shape was conceptually:
Here fps is video metadata, but it lived beside transfer controls in custom_output.
———
Why We Should Not Keep custom_output
Keeping custom_output and adding payload / metadata on top would leave two competing output protocols.
That would not really solve the architecture problem.
Actions are generated outputs. They are closer to image, video, audio, and text than to metadata.
If we kept this:
then the runtime still has to know that one generated modality is in output, while another generated modality is in custom_output.
That keeps the original inconsistency.
The old dict could contain:
These fields are not the same kind of thing.
Some are payload.
Some are public metadata.
Some are internal metadata.
Some are serving hints.
Some are model-specific temporary state.
A single custom_output dict cannot express those boundaries cleanly.
With custom_output, generic code has to keep doing logic like:
That pushes model-specific interpretation into DiffusionEngine, output_formatter, serving_chat, and serving_video.
The refactor goal is the opposite: model-specific details should be normalized by the model pipeline/postprocess, and generic runtime code should consume a stable structure.
Before, Cosmos3 had a separate action postprocess hook:
That existed because action data lived in custom_output, outside the normal postprocess path.
But action postprocess is still output postprocess. It should be handled inside the model-owned post_process, together with video/image/audio/text formatting.
If custom_output remains, then action still needs special runtime treatment.
For example:
is needed to convert model-space actions back to user-facing robot action format.
But it is not public API output.
Under custom_output, this field sits beside public fields like action_mode and domain_id.
With the new structure, it can be represented as:
Then the formatter strips
metadata["internal"]before producing API-facing output.That gives us a clear private/public boundary.
###After
The new model output shape is:
Top-level fields:
payload contains generated data.
Current supported payload keys include:
Meaning:
metadata contains information needed to interpret payload.
Current metadata groups include:
Concrete fields:
metadata["internal"]is only for model/postprocess-private state. It should not be exposed as public API metadata.Concrete New Cosmos3 Examples
Action-only output now becomes:
After postprocess, metadata["internal"] is stripped from public output.
Action + video output now becomes:
Transfer output now becomes:
Video + audio output can be represented as:
Text output can be represented as:
Runtime Behavior After Refactor
DiffusionEngine now only needs one normal postprocess path:
Then output_formatter normalizes the envelope.
The normalized internal object is:
Its fields mean:
This gives generic runtime code one way to consume all diffusion outputs.
Output runtime flow
Relations between TypeAlias:
DiffusionScalar
DiffusionShape
DiffusionTensorMap
torch.Tensor
list[DiffusionMetadataValue]
tuple[DiffusionMetadataValue, ...]
Mapping[str, DiffusionMetadataValue]
|
v
DiffusionMetadataValue
|
v
DiffusionMetadata
|
v
DiffusionMetadataMapping
Meaning:
DiffusionMetadata
= dict[str, DiffusionMetadataValue]
And:
DiffusionMetadataValue
= DiffusionScalar
| DiffusionShape
| DiffusionTensorMap
| torch.Tensor
| list[DiffusionMetadataValue]
| tuple[DiffusionMetadataValue, ...]
| Mapping[str, DiffusionMetadataValue]
DiffusionMetadataMapping is only the read-only view:
DiffusionMetadataMapping
= Mapping[str, DiffusionMetadataValue]
DiffusionScalar
bytes
torch.Tensor
np.ndarray
PIL.Image.Image
list[DiffusionPayloadValue]
tuple[DiffusionPayloadValue, ...]
Mapping[str, DiffusionPayloadValue]
|
v
DiffusionPayloadValue
|
v
DiffusionPayload
|
v
DiffusionPayloadMapping
Meaning:
DiffusionPayload
= dict[str, DiffusionPayloadValue]
And:
DiffusionPayloadValue
= DiffusionScalar
| bytes
| torch.Tensor
| np.ndarray
| PIL.Image.Image
| list[DiffusionPayloadValue]
| tuple[DiffusionPayloadValue, ...]
| Mapping[str, DiffusionPayloadValue]
DiffusionPayloadMapping is the read-only view:
DiffusionPayloadMapping
= Mapping[str, DiffusionPayloadValue]
DiffusionPayload
DiffusionMetadata
|
v
DiffusionOutputEnvelope
Meaning:
DiffusionOutputEnvelope
= {
"payload": DiffusionPayload,
"metadata": DiffusionMetadata,
}
This is the canonical post_process return shape.
DiffusionPayloadValue
DiffusionPayload
DiffusionOutputEnvelope
|
v
DiffusionPostprocessRawOutput
Meaning:
DiffusionPostprocessRawOutput
= DiffusionPayloadValue
| DiffusionPayload
| DiffusionOutputEnvelope
This means post_process_func may return one of three shapes:
single payload value
OR legacy payload dict
OR canonical envelope
DiffusionPayloadValue
DiffusionMetadata
|
v
DiffusionMultimodalOutput
Meaning:
DiffusionMultimodalOutput
= dict[str, DiffusionPayloadValue | DiffusionMetadata]
This is the type stored in:
OmniRequestOutput.multimodal_output
Minimal Summary Graph
Metadata branch:
DiffusionMetadataValue
|
v
DiffusionMetadata
|
v
DiffusionOutputEnvelope.metadata
Payload branch:
DiffusionPayloadValue
|
v
DiffusionPayload
|
v
DiffusionOutputEnvelope.payload
Accepted postprocess return types:
DiffusionPayloadValue
DiffusionPayload
DiffusionOutputEnvelope
|
v
DiffusionPostprocessRawOutput
Final serving multimodal output:
DiffusionPayloadValue
DiffusionMetadata
|
v
DiffusionMultimodalOutput
Test Plan
vLLM Version:
vLLM-Omni Commit:
Test Result
BEFORE SUBMITTING: read CONTRIBUTING.md and run the precheck-pr skill with the code agent for a self-check against project conventions.
(anything written below this line will be removed by GitHub Actions)