feat(minimax-h3): replace the t2va Flow-GRPO recipe with the measured configuration - #403
Open
celve wants to merge 12 commits into
Open
feat(minimax-h3): replace the t2va Flow-GRPO recipe with the measured configuration#403celve wants to merge 12 commits into
celve wants to merge 12 commits into
Conversation
The 768x768 Flow-GRPO recipe does not learn. At ~22k packed rows only 3 of the 10 SDE steps fit in replay and the group has to shrink, which is where the signal goes. On a 256x384 canvas the sequence is 3966 rows, so every step replays and the group can be 16 wide. Measured on two independent seeds against a decision rule fixed before either could be read: pooled +0.00234 reward/step, t=6.67, p<0.0001, with the seeds agreeing on the slope at p=0.598 and each significant alone. Two limits are written into the recipe header rather than left implicit. The rate decelerates inside the window (first half +0.00427, second half +0.00047, difference p=0.007), so it is not a linear rate to extrapolate. And the reward is imagebind+clap, neither of which scores artifacts, so a rising number is not evidence of better-looking video. Also corrects the sibling recipe's geometry comment, which asserted a 768px short-edge floor. No such floor exists in the resolver; it was inferred from an aspect-ratio helper's default, and it is what made the low-resolution setting look unavailable.
9 tasks
The header claimed the deceleration was what a bounded reward has to do. The sibling NFT recipe reaches 0.461 on the identical reward and prompt set and is still climbing, so ~0.29 is nowhere near the achievable ceiling and saturation explains nothing. The suspect is eta 0.7, whose stochastic rollouts score lower than the NFT recipe's deterministic ones, but that is untested. Also narrows the post-45 reading to one seed, where nothing is established.
The header named eta 0.7 as the suspect behind the gap to the NFT arm. The two recipes differ in objective, and eta is collinear with that choice: DiffusionNFT needs no log-prob and every NFT recipe here runs eta 0.0, while Flow-GRPO's loss is built on SDE log-probs and raises without them. The gap therefore supports only that NFT outperforms Flow-GRPO on this task. Isolating eta requires 0.7 against 0.3-0.5 within Flow-GRPO, which is noted.
0.7 is not a settled default for video: all three hunyuan_video15 recipes and wan21_t2v_videoalign run Flow-GRPO at 0.25, so that is the value with in-repo precedent for an isolating test. Also states that 124 frames cannot be traded for packed-row budget. Legal counts are 17n+5 and the 5s minimum duration puts the smallest legal value at 124, so unlike resolution this axis is closed.
Said neither reward component scores artifacts, which reads as a coverage gap a better reward would close. imagebind's coverage is in fact reasonable -- 10 of 124 frames as 5 adjacent pairs, three 224px crops each, against videopickscore's single frame. The gap is that it scores embedding alignment rather than quality, so a fringed clip that stays recognisable is aligned as well as a clean one at any coverage. That makes looking at frames the only thing that settles it.
Four knobs differ from the sibling and no run separates them; canvas and reward are perfectly collinear across every configuration anyone has tried. Also record that the plateau is a trough on seed 42 rather than a ceiling.
A 768 arm cannot hold the step count fixed: ten replayed steps need >100 GB/rank at 22k rows, which is why the sibling trains three of ten. The separable axis is the reward, at the cheap canvas.
imagebind's mode is unreachable through the composite, so it reduces to audio-video agreement with no text term, and the sibling NFT arm reached the degenerate solution that permits.
4 tasks
A PR body does not survive into the working tree, and the header documents the defect without putting the fix where someone editing this file will see it.
…measured The values on main -- 768x768, 3 of 10 SDE steps, group 4, lr 3e-4, two updates per batch -- have never been run by anyone; they arrived in 9e3e4b1 and the author of the branch it landed on disowns them. Keeping them beside a measured recipe invites running the unrun one, so this replaces rather than adds, and the path back to 768 stays in the header. Comments cut from 191 lines to 44, keeping only those naming a failure the reader cannot see from the file.
… replacement Upstream's recipe set master_dtype: fp32 and cast_forward_inputs: false; neither was present in the runs this file records, so restoring them would describe a configuration nobody measured. fsdp_wrap only casts params when master_dtype is set, so its absence leaves the trainable LoRA adapters at bf16. Co-authored-by: Cursor <cursoragent@cursor.com>
191 comment lines against 143 of config was more comment than config. What is left names a failure that is invisible from the file: the unsharded fp32 modules, the token_refiner LoRA collision, the small grad_norm, the omitted sde_indices, the frame-count floor, the host-RAM ceiling and the reward's missing text term. Co-authored-by: Cursor <cursoragent@cursor.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.
Replaces the H3 t2va Flow-GRPO recipe with the configuration that was actually run, and deletes the sibling I first proposed adding.
Why replace rather than add
The values currently on
mainhave never been run by anyone: 768x768,sde_indices: [0, 3, 6],samples_per_prompt: 4,learning_rate: 3e-4,num_updates_per_batch: 2. They arrived in9e3e4b13, and the author of the branch that commit landed on states those four knobs match none of theirs and that they have no measurements behind them. The nearest thing anyone did run at 768 — a larger group and a different reward — produced four null arms.So I cannot say the old configuration fails; I can say nobody has tried it, and that at 768 only 3 of 10 SDE steps fit in replay. Shipping it beside a measured recipe invites running the unrun one, which is why this replaces instead of adding a variant. The route back to 768 stays in the header.
mainvideopickscore+clapimagebind+clapTwo fields this drops
Upstream's file carries
master_dtype: fp32andcast_forward_inputs: false, and neither appears here, because neither was present in the runs below.fsdp_wrapcasts params only whenmaster_dtypeis set, so its absence leaves the trainable LoRA adapters at bf16 rather than fp32 masters — a real difference, and I am not claiming bf16 is better, only that it is what was measured. Flipping it blind would recreate the problem this PR is about, so it is in the test plan.The measurement
Two pods, 1x8 H20, same commit and recipe, differing only in seed. Pooled fit with per-seed intercepts and one shared slope, pre-registered before either seed reached the read point:
+0.00234 reward/step, t=+6.67, p<0.0001, 45 rollouts per seed, seeds agreeing at p=0.598. OLS and Newey-West HAC both p<0.0001. Levels move 0.205→0.288 and 0.173→0.313.
The canvas is what pays for the rest: 3966 packed rows against ~22k means every SDE step is replayable and the group can be 16 wide. All four knobs move together and no run separates them, so the result belongs to the bundle.
What this does not establish
ImageBindSpec.modedefaults to"audio_video", andT2AVCompositeScorerbuilds inner specs withinner_spec_cls()while propagating only("device", "batch_size", "frame_selection")— somodeis unreachable from a recipe andimagebindis exactlycos(audio_emb, video_emb).clapsees text but never a pixel. Nothing in this objective connects the prompt to the video.That is not hypothetical.
minimax_h3_t2va_nft.yamlon the same reward and canvas collapsed at rollout 100 to luma 2.01 / 2.14 of 255, 99.4% of pixels below 16, audio RMS down 9x and 28x — whileimagebindrose 0.158 → 0.316. Not a clamp artifact: of nonzero pixels p90=8, p99=16, max=47, so a tensor merely shifted out of the writer'sclamp(0, 1)window would have left a broad tail.Scoring those clips back through
ImageBindRewardScorer, both collapsed clips outscore both real ones (0.215 / 0.262 against 0.038 / 0.203), and constant black with silence scores 0.153 while constant gray with silence scores −0.0265. Direction established; magnitude not, at n=2 per condition.#404 fixes the reachability. This recipe does not set
mode— it cannot until #404 lands — so merging both still ships a reward with no prompt→video term. It also cannot simply be flipped afterwards:mode: "all"changes the reward, so the numbers above would stop describing the file. That needs its own run, and there is a note beside the weights saying so.Test plan
_target_resolves againstmain, includingimagebindin the reward registry; YAML parsescuda_alloc_gb25.24, stable to ±0.02advantage_std0.62–0.90,zero_std_group_ratio0 throughout256x384+videopickscore1.0 /clap0.0 — the only clean de-confound available, and a mechanism test:videopickscoreis a weak but real prompt→video anchor, so if anchoring is what separates flat from degenerate it should not collapsemaster_dtype: fp32restored — plausibly right, but it changes the trainable master dtype and no run here had iteta0.7 against 0.25 within Flow-GRPO — the only comparison that isolates eta; 0.25 has in-repo precedent in thehunyuan_video15recipesAlso corrected
The old header claimed H3 "was released for a 768px SHORT EDGE only" and that 768x768 is "the FLOOR". No such floor exists —
MiniMaxH3Geometry.resolveenforces multiple-of-32, aspect ratio, an area cap and the frame round-trip, and nothing else.num_frames: 124is a real floor: legal counts are 17n+5 and the 5 s minimum at 24 fps makes 124 the smallest, so frames cannot be traded for row budget the way resolution can.