[5/N][BREAKING][diffusion, rollout, tests] refactor: align diffusion prompt wire fields - #556
Draft
NancyFyong wants to merge 11 commits into
Draft
[5/N][BREAKING][diffusion, rollout, tests] refactor: align diffusion prompt wire fields#556NancyFyong wants to merge 11 commits into
NancyFyong wants to merge 11 commits into
Conversation
…quest contract Introduce a single typed request contract for diffusion/AR rollouts so the server-side request handling and condition-image parsing stop threading loose keyword arguments and re-deriving the same conventions in each strategy. Add MediaInput, PromptBundle, and OmniRolloutRequest, then have the shared generate template assemble one request object for both strategy preprocessors. Keep the public generate keyword RPC and the current engine prompt keys intact while forwarding multimodal processor metadata needed by Ref2VA. Remove ImageGenerationRequest because its two consumers only used its image alias lookup. Route those adapters through the shared parser instead, reject conflicting aliases and malformed containers, and validate duplicate or invalid media streams rather than silently overwriting them. Fail explicitly when AR receives diffusion-only prompt fields or diffusion receives an unsupported nonzero priority. Valid existing requests remain wire compatible; tree-wide canonical engine-key migration remains a later PR. Part of the verl-project#403 RFC, following merged PR verl-project#478. AI assistance (Pi coding agent / OpenAI Codex) was used for this change. Co-authored-by: OpenAI Codex Signed-off-by: NancyFyong <2742092809@qq.com>
…media kind Propagate the adapter-declared primary modality (media_kind) downstream so consumers stop guessing modality from tensor rank, and make V1 diffusion media dumps reliable. - V0 ray_diffusion_trainer: read declared media_kind (top-level non_tensor_batch first, tool_extra_fields fallback) and pass it into validation/wandb wrapping and generation dumping instead of is_video=ndim==5. - V1 trainer_base: delegate validation logging and generation writing to the shared V0 image/video/audio wandb+export paths via BaseRayDiffusionTrainer, pass media_kind/audio/audio_sample_rate/fps/validation_data_max_samples, and make background media dumps fail-open (futures keyed by global step, failures reported by _report_dump_failure instead of rethrown into training). Wire rollout_data_save_freq, rollout_data_max_samples, video_fps, and a thread-safe global-step snapshot. - TransferQueue (DiffusionAgentLoopWorkerTQ): preserve non-tensor media_kind and audio_sample_rate in the extra_fields envelope restored by diffusion_tq_batch_to_dataproto (tq_utils), and optional metadata arrays initialize with None via np.full instead of uninitialized np.empty for mixed present/missing rows. - rollout_media / DiffusionStrategy: keep only adapter-declared media_kind propagation; the Layout enum and media_layout were removed (moved to PR6). - tracking.wrap_val_samples_for_wandb: accept media_kinds, prefer declared kind over rank with rank as fallback, handle 5D batched video, and convert both video and image failures into warning + unavailable placeholder instead of raising into training. CPU-validated: 74 targeted tests across strategy, tracking-media, MiniMax contract, V1 media dump/replay, and agent-loop correctness; py_compile and whitespace clean. AI assistance (Pi coding agent / OpenAI Codex) was used for this change. Co-authored-by: OpenAI Codex Signed-off-by: NancyFyong <2742092809@qq.com>
…casting Compare NumPy and Torch payloads by shape and contents instead of scalar truth conversion, preserving equivalent aliases and rejecting broadcast-only matches. AI assistance (Pi coding agent / OpenAI Codex) was used for this change. Co-authored-by: OpenAI Codex <noreply@openai.com> Signed-off-by: NancyFyong <2742092809@qq.com>
…onsumer boundaries Validate modality and supported auxiliary streams before consumption. Forward generated media to colocated reward scorers, reject conflicting sources, and contain filesystem and logger failures without swallowing modality errors. AI assistance (Pi coding agent / OpenAI Codex) was used for this change. Co-authored-by: OpenAI Codex <noreply@openai.com> Signed-off-by: NancyFyong <2742092809@qq.com>
…prompt wire fields Use one diffusion token/media contract across adapters without moving multimodal inputs away from the pinned upstream runtime readers. Keep the initial AR-stage token entrance explicit and conflict-checked. AI assistance (Pi coding agent / OpenAI Codex) was used. Co-authored-by: OpenAI Codex <noreply@openai.com> Signed-off-by: NancyFyong <2742092809@qq.com>
Match the pinned engine preprocessing gate instead of inferring AR preprocessing from the number of stages. Regression cases cover both stage kinds with one and two stages. AI assistance (Pi coding agent / OpenAI Codex) was used. Co-authored-by: OpenAI Codex <noreply@openai.com> Signed-off-by: NancyFyong <2742092809@qq.com>
NancyFyong
changed the title
2 tasks
Keep the three processor-hook regression tests when removing the obsolete ImageGenerationRequest tests. Update the I2I guide to the shared parser and remove its redundant Boogu fallback. Validation: 37 targeted CPU tests and 1140 full CPU tests passed; manual lint, format, mypy, config and sanity checks passed. AI assistance (Pi coding agent / OpenAI Codex) was used. Co-authored-by: OpenAI Codex <noreply@openai.com> Signed-off-by: NancyFyong <2742092809@qq.com>
…umers Merge the restored processor-hook coverage, current I2I parser docs and redundant-fallback fix from PR3 without rewriting the published stack. Validation: 1181 CPU tests and manual sanity checks passed. AI assistance (Pi coding agent / OpenAI Codex) was used. Co-authored-by: OpenAI Codex <noreply@openai.com> Signed-off-by: NancyFyong <2742092809@qq.com>
…erage Carry PR3 review repairs through the request-wire stack while retaining published history. Validation: 1199 CPU tests and manual sanity checks passed. AI assistance (Pi coding agent / OpenAI Codex) was used. Co-authored-by: OpenAI Codex <noreply@openai.com> Signed-off-by: NancyFyong <2742092809@qq.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR5 of #403, stacked on #481 (
0c4c40e). This PR is based onmainfor the upstream review, so its displayed diff includes #480/#481 until those merge. The PR5-only diff is0c4c40e..HEAD.OmniRolloutRequestto one documented diffusion wire prompt:prompt_ids, negative IDs/masks, top-level media and processor kwargs, and extra-encoder IDs underextra_args.extra_argsjust to satisfy the incomplete upstream TypedDict.prompt_token_idsonly when the first stage's declaredstage_typeis not diffusion, matching the pinned engine's preprocessing gate. Stage count is not a proxy for stage type. The shared reader rejects conflicting spellings. ARStrategy and the public Ray keyword API are unchanged.Compatibility
This changes the internal diffusion prompt contract for external custom adapters: read
prompt_idsthroughprompt_ids_from_payload, and read extra-encoder IDs underextra_args. Existing direct callers using AR token spelling remain accepted at the shared boundary. The conflict-checking historical condition-image parser remains a compatibility entrance; the server emits only one media location.No upstream package, lifecycle code, training tensor keys, or model/tokenizer weights are modified. This is request-side work only; named output artifacts/layout migration belongs to PR6.
Duplicate-work check
Read #403 with comments and searched open PRs for
403 in:body,wire schema, andmedia artifacts. #480/#481 are the intended lower layers; no open PR implements this all-adapter wire migration. This continues, rather than duplicates, that series.Tests
Environment: pinned verl
fefb080, vllm-omnided893462(0.28).New tests exercise canonical IDs at real Qwen/Boogu, MiniMax, Bagel, LTX and SD3 adapter extraction boundaries without loading weights; strategy tests cover both first-stage types independently of one/two-stage topology, media, negative/extra IDs and processor kwargs. The new stage-selection regression failed in two cases before the fix and passes after it. Existing batching/cache/step tests also pass.
Ruff 0.12.2 check/format, mypy 1.17.0 (repository configuration), generated-config verification, docs-time/docstrings/license/device-API/DataProto/structure checks, compileall, and whitespace checks passed. Hooks were run directly because the known host Git/pre-commit incompatibility prevents the launcher; mypy's repository-wide
ignore_errorslimits that evidence.Upstream GPU CI remains pending. New local real-engine verification on the #557 stack exercises this wire path for Qwen FlowGRPO/DPO/NFT (including step execution), dual/mix, multi-encoder SD3, Boogu, Wan, LTX, MiniMax H3 NFT/FlowGRPO and single-stage Bagel. Qwen/SD3/Boogu packed-request checks verify multiple IDs in producer context. H3 NFT V0 and real V1/TQ complete a training step with CLAP/ImageBind and audiovisual export. See #557 for the complete current matrix, checkpoint/mode limitations and image-edit evidence. The AR entrance discriminator is covered by CPU topology tests, not a claimed multistage AR GPU pass. No unrelated GPU/Ray task was stopped.
Review follow-up (2026-09-08)
Current head
dd3f453merges the #480 review repairs through #481 without rewriting published history. The three processor-hook tests are restored, the I2I parser guide is current, and Boogu's redundant fallback is removed. Full CPU command above: 1199 passed; manual lint/config/sanity checks also passed. This test/docs/redundant-fallback follow-up does not claim a new GPU run.Disclosure
AI assistance (Pi coding agent / OpenAI Codex) was used. Human review of every changed line and relevant validation are required before merge; this draft does not assert that the submitter has already completed that review.