Repository navigation
feat(minimax-h3): carry FL2VA keyframe condition rows through rollout, replay and NFT - #584
Open
MinhaoLi0318 wants to merge 2 commits into
Open
MinhaoLi0318 wants to merge 2 commits into
MinhaoLi0318 wants to merge 2 commits into
Conversation
…, replay and NFT Step 1a of Tencent-Hunyuan#391 item 4 (FL2VA). Adds MiniMaxH3KeyframeCondition, the per-sample fp32 noised condition rows, their anchors and unpadded text token tags, as an optional `keyframes` slot of MiniMaxH3Conditions. build_t2va_layout becomes build_layout(geometry, conditions) over the vendored [text | keyframe conditions | audio | video] builder, and MiniMaxH3DiffusionStep.predict_noise, the one forward that generate, replay and predict_noise_at_step share, prepends the carried rows and returns target-row velocity only. Trajectories, SDE log-probs and the NFT target therefore stay target-only. trim_text_padding now keeps every non-text slot instead of rebuilding the container from text alone. A batch with keyframes carries an entry for every sample. A t2va sample's entry is anchors (), (0, 96) fp32 rows and all-text tags: build_layout turns it into exactly the t2va layout and predict_noise prepends nothing, so mixed t2va/fl2va batches stay aligned through concat, slice and select. MiniMaxH3Conditions.__post_init__ rejects a keyframes sample count that differs from text, so construction, concat, slice, select and from_dict of the typed container fail closed. Part.concat of Part.conditions dicts drops the keyframes key of a sample without one, so to_dict stores text as MiniMaxH3KeyframedText exactly when keyframes is set: a mixed Part.concat raises when a keyframe sample comes first, and otherwise from_dict rejects the result and every slice, select or chunk of it. Nothing produces keyframes yet: step 1b is the producer and must emit an entry for every sample or for none. Condition rows keep running at the target video timestep until step 2. t2va is bit-identical to 0832c3b. Signed-off-by: MinhaoLi0318 <lminhao039@gmail.com>
…ur of Tencent-Hunyuan#582 Tencent-Hunyuan#582 makes Batch.concat raise when a non-empty item lacks a per-sample dict key, so a Part.concat that mixes samples with and without the keyframes key now raises in the generic path, in every sample order. The MiniMaxH3KeyframedText binding guarded exactly that case, so it is removed: to_dict and from_dict return to main's one-liners, and a producer may build Part.conditions by hand instead of through to_dict. MiniMaxH3Conditions.__post_init__ still rejects a keyframes sample count that differs from text, which Tencent-Hunyuan#582 does not check, and build_layout keeps its anchor, tag and row checks. Signed-off-by: MinhaoLi0318 <lminhao039@gmail.com>
MinhaoLi0318
force-pushed
the
feat/minimax-h3-fl2va-conditions
branch
from
October 13, 2026 14:12
f3b8b8e to
6dde41e
Compare
Contributor
Author
|
#582 is in main ( AI assistance: drafted with Claude Code; I reviewed it before posting. |
This branch has not been deployed
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
Step 1a of #391 item 4 (FL2VA: first, last and first+last keyframe conditioning for MiniMax-H3). Rollout stores the noised keyframe condition rows as data; replay and the DiffusionNFT forward reuse them and never recompute them.
Why carry the rows: diffusers 0.40.0 and vLLM-Omni 0.28 draw the condition noise and resize the second keyframe differently, so train-side recomputed rows would silently break the replay log-prob ratio. Carried rows are byte-identical to the rollout rows, as @CjhHa1 asked for step 1 on #391.
Scope: t2va stays bit-identical to main
6c828f5, and to main0397ddbonce merged (Test Plan 5). No code path setskeyframesyet: this PR is the consumer, and step 1b adds the producer (keyframe encode, conditioner prompt, data path), per the 1a / 1b split I proposed on #391; I can fold 1b into this branch if you prefer one PR. Condition rows still run at the target video timestep until step 2.Changes (6 files in
unirl/models/minimax_h3/, +125/-23):keyframesslot (MiniMaxH3KeyframeCondition) onMiniMaxH3Conditions, one entry per sample: fp32 rows[K * rows_per_frame, 96], anchors, unpadded text-token tags. t2va samples in a keyframe batch carry an empty entry.__post_init__checks alignment, and aPart.concatthat mixes samples with and withoutkeyframesraises in fix(tensor): fail closed when batch concat or DP dispatch sees inconsistent shards #582'sBatch.concatcheck instead of shifting rows.build_t2va_layout→build_layout(geometry, conditions)with fail-closed checks.generate,replayandpredict_noise_at_stepshare onepredict_noisethat prepends the carried rows and returns target-only velocity.trim_text_paddingkeeps the other slots.minimax_h3/README.mdGotchas;vendor/__init__.pyre-exportsMINIMAX_H3_VIDEO_TAG(vendored files untouched).Related Issue
Part of #391 (item 4, step 1a).
Prior art: verl-omni #401 (MiniMax H3 FL2VA DiffusionNFT, merged 2026-08-24) and #448 (FL2VA FlowGRPO, merged 2026-08-31) also carry the noised keyframe rows from rollout into the training forward and score only the target rows; #401's trainer already runs condition rows at
max(t, 0.999), which is step 2 here.Test Plan
Commits
8976f4b(f3b8b8erebased,git range-diff1: f3b8b8e = 1: 8976f4b) and6dde41e(drops the binding; see Reviewer Notes) on6c828f5. macOS arm64, Python 3.12.14, torch 2.13.0 CPU, diffusers 0.40.0. CPU plus one CPU→MPS→CPU round trip; no GPU.One-off CPU harness, not committed (AGENTS.md §5), available on request: real
MiniMaxH3Bundleand stage (CPSSDE, eta 0.7), tiny random-config vendored transformer, 64x96x124 canvas; every transformer kwarg, log-prob and output compared withtorch.equalplus dtype and shape. All steps exit 0:run_fl2va_582.pyis the earlierrun_fl2va.pywith the binding checks replaced by checks against #582'sBatch.concat.run_shim.pyre-creates_align_track_to_model, which #592 folded intoBaseTrainStack._align_track_inputs(same body).predict_noise_at_step, padded micro-batch, a t2va sample inside mixed t2va/fl2va Parts). Comparator negative control: 3/3 injected changes flagged.207 passed, 0 failed: first, last, first+last throughPart.concat→ pickle →TensorTransport→ CPU→MPS→CPU → slice/select →replay→predict_noise_at_step; sparse SDE window; bf16; mixed batches; fail-closed inputs. Parity negative controls (re-drawn noise, fp16 round trip, all-text tags, swapped anchors, main'strim_text_padding) each break parity; the smallest |Δlogp| among them is 1.5e-13, so the check compares bytes. EveryPart.concatthat mixes samples with and withoutkeyframes(missing first, middle or last; padding path; DP gather shape) raises in fix(tensor): fail closed when batch concat or DP dispatch sees inconsistent shards #582's check. With the__post_init__guard off, its 4 construction and slice cases stop raising and the 4 typed-concat cases still raise in fix(tensor): fail closed when batch concat or DP dispatch sees inconsistent shards #582's check; with fix(tensor): fail closed when batch concat or DP dispatch sees inconsistent shards #582's check off,[None, f1, f2, f3]silently shifts rows. Before fix(tensor): fail closed when batch concat or DP dispatch sees inconsistent shards #582,f3b8b8eon0fe4f99gave210 passed, 0 failed, including the binding's own checks.68 passed, 0 failedagainst the vendored builder, diffusers 0.40.0, vLLM-Omni v0.28.0 and vLLM-Omni main0ad67f1d8, four canvases.pr500_bytes.py10 passed, 0 failed(|Δlogp| ≤ 1.03e-13);run_fl2va_582.py207 passed, 0 failed.0397ddb(2026-10-16) → its 17 new commits touch nothing inminimax_h3/ortypes/conditions/; fix(distributed,reward): access typed handles, batches, and scorer specs directly #604 rewrotebatch.pybut kept fix(tensor): fail closed when batch concat or DP dispatch sees inconsistent shards #582's missing-value check (batch.py:144-146).git merge-treewith6dde41eis clean. On the merge result: t2va vs0397ddb791 equal / 0 differ, negative control 3/3; FL2VA205 passed, 0 failed(fix(distributed,reward): access typed handles, batches, and scorer specs directly #604 removedBatch.repeat_interleave, so its 2 checks are skipped; the shim follows fix(distributed,reward): access typed handles, batches, and scorer specs directly #604'sto_devicebody); layout68 passed, 0 failed. With perf(minimax-h3): batch replay timesteps #500 on top:pr500_bytes.py10/0, FL2VA 205/0. With fix(models): read typed bundle/pipeline contracts directly and drop falsy fallbacks #605 on top: t2va 791/0 vs0397ddb+ fix(models): read typed bundle/pipeline contracts directly and drop falsy fallbacks #605, FL2VA 205/0, layout 68/0.Parity uses synthetic noised rows (1a has no producer); 1b re-runs it through
MiniMaxH3Pipeline.generate. The DiffusionNFT loss itself was not run.--cfg job --resolve.Compatibility / Risk
build_t2va_layoutrenamed tobuild_layout(no other caller); new exportMiniMaxH3KeyframeCondition.{"text": ...}dict still works.Part.conditionscan come fromMiniMaxH3Conditions.to_dictor be built by hand.1 - sigma, the released model atmax(1 - sigma, 0.999)(diffusers 0.40.0before_denoise.py). The README forbids enabling fl2va before step 2, which must fix both main'srow_timestep_planand perf(minimax-h3): batch replay timesteps #500's batched plan.torch.stackand onetorch.catper forward.minimax_h3/README.md(add/add, no contradiction); whichever lands second merges the READMEs. fix(models): read typed bundle/pipeline contracts directly and drop falsy fallbacks #605 also editsminimax_h3/diffusion.py,packing.pyandpipeline.pyand merges cleanly (Test Plan 5). fix(tensor): fail closed when batch concat or DP dispatch sees inconsistent shards #582 is in main.Duplicate-Work Check
Re-checked 2026-10-11 on the public GitHub pages and with
git ls-remote upstream:FL2VA: feat(minimax-h3): rework the vLLM-Omni H3 rollout onto 0.27.0rc1 #408 only (closed, vLLM-Omni 0.27.0rc1 rollout). PRs matchingkeyframe: none. Issues matchingfl2vaorkeyframe: [Roadmap] MiniMax-H3 critical path #391 only.minimax(7), heads fromrefs/pull/<N>/head, merged with this commit bygit merge-tree: perf(minimax-h3): batch replay timesteps #500 (d3bdb23) clean; feat(minimax-h3): DiffusionNFT on the vLLM-Omni rollout backend at 32-GPU scale #420 (521c217) adds only the README add/add; feat(minimax-h3): vLLM-Omni 0.28 rollout backend for H3 FlowGRPO #378 (b21fca7), perf(minimax-h3): shard the video VAE decode across the SP group #469 (363d5ba), feat(trainer): opt-in row-level reward dispatch for diffusion #517 (fca5a55), feat(flux2_klein): support multiple reference images end to end #351 (acabb46), perf(minimax-h3): enable expandable allocator segments for trainside training #583 (81a011d) touch none of these files and gain no new conflict.minimax_h3/ortypes/conditions/, or mentions keyframe/fl2va in its commit messages. fix(tensor): fail closed when batch concat or DP dispatch sees inconsistent shards #582 (07dec54) changesdistributed/tensor/batch.py; see Reviewer Notes.Re-checked 2026-10-13 after the rebase: PRs matching
keyframeorFL2VAare still only #408 and this one; #391 has no new comments. New since 2026-10-11: draft #593 (da8d285, MiniMax-H3 block training) addsminimax_h3/README.md, an add/add with this PR. #582 and #583 are now in main.Re-checked 2026-10-16 against main
0397ddb: PRs matchingkeyframeorFL2VAare still only #408 (closed) and this one; issues matchingfl2vaorkeyframeare #391 and the Q2 roadmap #25 (last updated 2026-09-12); #391 has no new comments. Besides this one, 8 open PRs matchminimax; the only new one is #605 (5545bcb), which editsminimax_h3/{diffusion,packing,pipeline}.pyand merges cleanly with this head, and the other heads are unchanged. Open PRs #589-#609 mention neither keyframe nor fl2va in their commit messages.git merge-treeof this head with each of the other 64 open PRs: only #420 and #593 gain a conflict (the README add/add).Reviewer Notes
max(1 - sigma, 0.999)in both main's and perf(minimax-h3): batch replay timesteps #500's plan, assertions, and an engine-side row-count check. Alternative: fold it into 1b with perf(minimax-h3): batch replay timesteps #500.keyframesslot (they exist only with keyframes). If you prefer the text side, now is the cheapest time to move them.0fe4f99the generic dict concat dropped a missing key, so[None, f1, f2, f3]would give 4 text rows and 3 entries and shift every later sample. The first commit guarded this with a binding (MiniMaxH3KeyframedTextplus afrom_dicttype check). fix(tensor): fail closed when batch concat or DP dispatch sees inconsistent shards #582 (790b5ec) makes the generic concat raise on a value missing in a non-empty item, for every model, so6dde41edrops the binding (2 files, +7/-37) andto_dict/from_dictare main's again.__post_init__'s alignment check stays: it also covers direct construction and a keyframes count that differs from text.pr-reviewskill; independent reviewer Sonnet 5.5 (claude-sonnet-5-5), fresh context, 3 rounds; author Claude Opus 5.5. Last round: no P0/P1/R. Since then: README wording, the rebases onto0fe4f99and6c828f5(git range-diff:1: 8f4bd75 = 1: f3b8b8e, then1: f3b8b8e = 1: 8976f4b), and6dde41e, which removes the binding after fix(tensor): fail closed when batch concat or DP dispatch sees inconsistent shards #582 merged; a fourth independent round (Sonnet 5.5, fresh context) on6dde41ereported no P0/P1/R. Fixed: mixed-batch alignment (the binding and itsfrom_dictunwrap, now replaced by fix(tensor): fail closed when batch concat or DP dispatch sees inconsistent shards #582's check), tuple-only anchors. Kept: target-row velocity slice, README despite feat(minimax-h3): DiffusionNFT on the vLLM-Omni rollout backend at 32-GPU scale #420, generic_concat_valueunchanged in this PR.generate, GPU check.Checklist
AI assistance: drafted with Claude Code; I reviewed it before posting.