From 320f8a1f8d2170ea2855e395b6fcc54e91bee804 Mon Sep 17 00:00:00 2001 From: SolitaryThinker Date: Sat, 2 May 2026 00:22:19 -0700 Subject: [PATCH] [bugfix] magi-human: pack video tokens channel-major to match upstream FV's _img2tokens was packing video latent patches as spatial-major '(pT pH pW C)' (channels innermost), but the DiT's video_embedder Linear weight was trained on upstream's channel-major '(C pT pH pW)' layout. Upstream's MagiDataProxy uses UnfoldNd, which is implemented via a grouped-channel conv whose output reshape is channel-major (in_channels * kernel_size_numel ordering, channels slowest). The spatial-major layout silently permuted the in-features of every video token, scrambling the entire video feature representation and producing pure colorful-blob noise from the basic example. The pipeline parity test could not catch this because it imports FV's build_packed_inputs for the upstream side too, so both sides consumed equally-permuted tokens and agreed on garbage at production scale (T=26 frames, ~3120 video tokens), while reporting healthy ~0.5%/step drift on the tiny synthetic (2,6,6) latent. Fix is a one-character change in the einops rearrange pattern. unpack_tokens stays spatial-major because the DiT's final_linear_video is trained to emit (pT pH pW C), matching upstream's SingleData.depack_token_sequence at data_proxy.py:220-228. Validated end-to-end: examples/inference/basic/basic_magi_human.py now produces coherent video at the standard 480x256 / 32-step / seed 42 prompt -- woman on a park bench reading a book under green trees, matching prompt. Output mp4 size dropped from ~932KB (incompressible noise) to ~222KB (coherent compressible video). Closes magi-human OQ-6. Notes for follow-up: - OQ-11 (NEW): test_magi_human_pipeline_parity.py:222 should drive the upstream side through real MagiDataProxy.process_input so the parity test catches packing-layout regressions natively. --- .../magi_human/stages/latent_preparation.py | 15 +++++-- tests/local_tests/magi-human.md | 39 ++++++++++++++++++- 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/fastvideo/pipelines/basic/magi_human/stages/latent_preparation.py b/fastvideo/pipelines/basic/magi_human/stages/latent_preparation.py index 5b0a11e5b..7636756b6 100644 --- a/fastvideo/pipelines/basic/magi_human/stages/latent_preparation.py +++ b/fastvideo/pipelines/basic/magi_human/stages/latent_preparation.py @@ -75,15 +75,22 @@ def _pad_or_trim_dim1(t: torch.Tensor, target: int) -> tuple[torch.Tensor, int]: def _img2tokens(x_t: torch.Tensor, t_patch: int, patch: int) -> torch.Tensor: - """Pack a video latent [B, C, T, H, W] -> [B, L, C * t_patch * patch^2].""" + """Pack a video latent [B, C, T, H, W] -> [B, L, C * t_patch * patch^2]. + + Per-token feature ordering is channel-major ``(C pT pH pW)``: the DiT's + ``video_embedder`` weight was trained on the layout produced by + upstream's grouped-conv ``UnfoldNd`` packer (channel slowest, patch + elements fastest). Spatial-major ``(pT pH pW C)`` silently permutes the + in-features and produces noise output. Asymmetric with + ``unpack_tokens`` which uses ``(pT pH pW C)`` to match + ``final_linear_video``'s trained output layout. + """ B, C, T, H, W = x_t.shape assert T % t_patch == 0 and H % patch == 0 and W % patch == 0, ( f"Latent dims {T,H,W} must divide ({t_patch}, {patch}, {patch})") - # Rearrange so each (t_patch, patch, patch) block becomes one token with - # concatenated channels — mirrors the reference `UnfoldNd(stride=kernel)`. return rearrange( x_t, - "B C (T pT) (H pH) (W pW) -> B (T H W) (pT pH pW C)", + "B C (T pT) (H pH) (W pW) -> B (T H W) (C pT pH pW)", pT=t_patch, pH=patch, pW=patch, diff --git a/tests/local_tests/magi-human.md b/tests/local_tests/magi-human.md index 6d93e97f6..5778aa61c 100644 --- a/tests/local_tests/magi-human.md +++ b/tests/local_tests/magi-human.md @@ -401,6 +401,42 @@ fp32 parity now works end-to-end on the FV side (model is dtype-agnostic in forw OQ-9 (NEW): upstream `daVinci-MagiHuman/inference/model/dit/dit_module.py` has hardcoded `.to(torch.bfloat16)` casts at lines 619, 507, 650, 694, 696. For full fp32 parity validation, these would need to be patched in the local clone OR a flag added upstream. Tracked as low-priority follow-up; affects only fp32 parity testing, not production. +### Wave 14 (2026-05-02): upstream E2E coherent vs FV E2E noise (REAL bug confirmed) + +Ran the upstream `daVinci-MagiHuman` pipeline end-to-end with the same prompt + seed (42) + steps (32) + resolution (480x256) used by `examples/inference/basic/basic_magi_human.py`. Required installing `magi_compiler` from the local subdir, `alias_free_torch`, and downgrading `diffusers` per upstream's pinned version. + +**Result**: upstream produces a **coherent** video — young woman in a pink shirt reading a red book on a park bench surrounded by green trees, matching the prompt. Reference at `/tmp/opencode/upstream_magi_base_4s_480x256.mp4` (frames at `/tmp/opencode/upstream_frame_*.png`). FV produces **pure colorful-blob noise** at the same configuration (`outputs_video/magi_human_basic/output_magi_human_*.mp4`). + +**This invalidates the Wave 13 "structural / no real bug" verdict** for OQ-6 and reopens it. The bug is in code that production exercises but the parity test bypasses — parity test still passes (~0.5% per-step drift on (2,6,6) tiny synthetic latents) yet production produces noise on real (26,16,30) latents with real text encoding. + +#### Falsified candidates so far + +1. **T5-Gemma fp16 cast (Candidate A)**. Upstream `t5_gemma_model.py:24-27` casts `outputs["last_hidden_state"].half()` (bf16→fp16) before pad/trim → fp32; FV keeps bf16 → fp32 (`fastvideo/pipelines/basic/magi_human/pipeline_configs.py:t5gemma_postprocess_text`). Parity test bypasses this because it uses FV's encoder for both upstream and FV sides (`tests/local_tests/magi_human/test_magi_human_pipeline_parity.py:118-147`). Applied `outputs.last_hidden_state.to(torch.float16)` in the postprocess function and reran the basic example → **still pure noise**, visually identical to before. Reverted. + +2. **Local-window video→video attention (Candidate B)**. Upstream `MagiDataProxy.process_input` returns 5 args including `local_attn_handler` (`daVinci-MagiHuman/inference/pipeline/data_proxy.py:319-382`); FV's `MagiHumanDiT.forward` only takes `(x, coords, mm)` and uses full SDPA. **Verified to be inert for the base model**: upstream `local_attn_layers` config defaults to `[]` for the base BR pipeline (`daVinci-MagiHuman/inference/common/config.py:71`); only the SR_1080 pipeline sets non-empty layer indices (lines 229-241). Base-model upstream uses `flash_attn_with_cp` (full attention) at `dit_module.py:644-645`, equivalent to FV's full SDPA. + +#### Root cause + fix (Oracle, 2026-05-02) + +**Bug**: FV's `_img2tokens` packed video latents as **spatial-major** `(pT pH pW C)` (channels innermost) at `fastvideo/pipelines/basic/magi_human/stages/latent_preparation.py:84`. Upstream's `MagiDataProxy.process_input` uses `UnfoldNd(...)` at `daVinci-MagiHuman/inference/pipeline/data_proxy.py:287-317`, which is implemented via a grouped convolution (`groups=in_channels`) that reshapes to `(batch, in_channels * kernel_size_numel, -1)` (`unfoldNd/unfold.py:66`) — i.e. **channel-major** `(C pT pH pW)` (channels slowest). The DiT's `video_embedder` (`Linear(192, 5120)`) was trained on the channel-major layout. Spatial-major input silently permutes the in-features of every video token, scrambling the entire feature representation and producing pure noise. + +**Why parity test passed**: `test_magi_human_pipeline_parity.py:222` imports FV's `build_packed_inputs` for the upstream side too, so both sides ate the same FV-spatial-major tokens and agreed on equally-wrong inputs. Production faces real DiT weights and breaks. + +**Fix**: One-character rearrange-string change in `_img2tokens`: +```diff +- "B C (T pT) (H pH) (W pW) -> B (T H W) (pT pH pW C)" ++ "B C (T pT) (H pH) (W pW) -> B (T H W) (C pT pH pW)" +``` + +`unpack_tokens` keeps spatial-major `(pT pH pW C)` because the DiT's `final_linear_video` was trained to emit that layout, mirroring upstream's `SingleData.depack_token_sequence` at `data_proxy.py:220-228`. + +**Validation**: Reran `examples/inference/basic/basic_magi_human.py` at the standard 480x256 / 32 step / seed 42 prompt. Output is **coherent video** matching the prompt — woman in teal sweater on a wooden park bench reading a book, green trees, sunny park scene. Output mp4 size dropped from ~932 KB (incompressible noise) to ~222 KB (coherent video). Frame samples at `/tmp/opencode/channelmajor_frame_*.png`. + +**OQ-6 RESOLVED-FOR-REAL** (Wave 14). + +#### Parity test fidelity follow-up (separate issue) + +The pipeline parity test should be updated to use upstream's *real* `MagiDataProxy.process_input` for the upstream side (instead of importing FV's `build_packed_inputs`), so it can catch this class of "both sides use FV's helper, both consume scrambled tokens, parity passes" bypass in the future. Tracked as OQ-11. + ### Potential mitigations (not investigated this session) - Run sensitive ops (MM-layer pre-norm, attention) in fp32 instead of bf16. @@ -432,7 +468,8 @@ with hard rules around no-source-residue cleanup. | OQ-3 | **Audio quality regression metric.** Mel-spectrogram L1 / multi-resolution STFT regression deferred per `tests/local_tests/stable-audio.md` precedent. | DEFERRED | | OQ-4 | **`_find_base_shard_dir` is fragile across HF-cache configurations.** Wave 1 fixed the loader with `snapshot_download(repo_id, allow_patterns=['base/*.safetensors'])` fallback in 3 files. `MAGI_HUMAN_BASE_SHARD_DIR` still works as an override but is no longer required. | RESOLVED | | OQ-5 | **Basic-example output mp4 visual quality is impressionistic at 256x448.** Root cause identified: OQ-6 (pre-existing compounding bf16 drift over the 32-step denoise loop). Wave 2-3 investigation confirmed the 4-step pipeline parity shows 18.85x compounding ratio vs expected 4x linear. See OQ-6 for full details and mitigation candidates. | RESOLVED-ROOT-CAUSE-IDENTIFIED (see OQ-6) | -| OQ-6 | **Pre-existing compounding bug in MagiHumanDiT denoising loop (HIGH PRIORITY).** 4-step pipeline parity shows video diff_mean=1.30 vs 1-step 0.069, a ratio of 18.85x (expected ~4x linear). Bug pre-exists in commit 620aaf41 (original magi port); confirmed via `git revert` bisect (4-step diff_mean=1.20 with all Wave 1 reverts). Single-forward DiT drift (diff_max=0.057) is bf16-noise-floor: cumulative ~1e-3 per-layer over 40 layers, consistent with random-walk accumulation. Wave 3 ruled out: PackedExpertLinear routing (A/B patch showed zero change), expert chunk ordering (bit-exact direct test), conversion script (bit-exact). Wave 7 partial fix: FV's `_MAGI_HUMAN_NEGATIVE_PROMPT` was missing the audio-quality + speech-delivery blocks present in upstream `video_generate.py:222-224`; audio CFG amplified the missing-block delta 5x, explaining the observed step-1 audio amplification of ~3x. Fix applied at `presets.py`. Wave 8 additional production fixes: tokenizer pre-padding (t5gemma.py), resolution defaults (presets.py + latent_preparation.py), silent audio fallback (audio_decoding.py). ALL production-facing root causes now identified and fixed. Parity-test 4-step compounding is a separate phenomenon — inherent FlowUniPC multistep scheduler amplification of per-call bf16 noise; NOT a code bug. fp32 sensitive ops or scheduler change would be needed to materially reduce parity-test compounding. | RESOLVED-PRODUCTION; bf16 amplification floor TRACKED separately | +| OQ-6 | **Video patch packing was spatial-major instead of channel-major.** Wave 14 (2026-05-02) ran upstream E2E and got coherent output; FV produced pure noise at same config. Oracle triage identified the bug in `_img2tokens` rearrange order: FV used `(pT pH pW C)` (spatial-major) but the DiT's `video_embedder` Linear weight was trained on the channel-major `(C pT pH pW)` layout that upstream's `UnfoldNd` (grouped-conv reshape, `unfoldNd/unfold.py:66`) produces. The pipeline parity test imported FV's `build_packed_inputs` for both sides at `test_magi_human_pipeline_parity.py:222`, so it consumed equally-permuted tokens on both sides and reported agreement on garbage. Fixed in `latent_preparation.py:_img2tokens` by changing the einops pattern from `(pT pH pW C)` to `(C pT pH pW)`. Validated end-to-end: `examples/inference/basic/basic_magi_human.py` now produces coherent video matching the prompt (woman on park bench reading a book, green trees). Earlier waves' production-side fixes (negative prompt, tokenizer padding, resolution defaults, silent-audio fallback) all still stand. | RESOLVED — Wave 14 | +| OQ-11 | **Pipeline parity test imports FV's `build_packed_inputs` for the upstream side.** `tests/local_tests/magi_human/test_magi_human_pipeline_parity.py:222` calls FV's packer for both sides instead of upstream's real `MagiDataProxy.process_input`. This let the channel-major-vs-spatial-major bug (OQ-6, Wave 14) sit silent for weeks because both sides agreed on the wrong layout. Update the parity test to drive the upstream side through `MagiDataProxy.process_input` so future packing-layout regressions are caught at parity time, not at production E2E. | TRACKED FOLLOW-UP | | OQ-7 | **Wan VAE shared fp32 op-order drift (MEDIUM PRIORITY).** FV uses `z * std + mean` at decode normalization; upstream uses `z / (1/std) + mean`. Bitwise non-equivalent in fp32. Affects all Wan-family pipelines (`fastvideo/configs/pipelines/wan.py`, `turbodiffusion.py`, `longcat.py`, magi-human). Magi VAE test loosened to `atol=1e-3, rtol=1e-3` (Wave 4) to defer. Tighten back to `atol=1e-4` once the Wan VAE op-order fix lands. Fix should be validated against Wan2.1, Wan2.2, and magi-human. Estimated 0.5-1 day to fix and validate. | TRACKED FOLLOW-UP | | OQ-9 | **Upstream `dit_module.py` hardcoded bf16 casts block full fp32 parity validation.** `daVinci-MagiHuman/inference/model/dit/dit_module.py` has hardcoded `.to(torch.bfloat16)` casts at lines 619, 507, 650, 694, 696. FV's DiT forward is now dtype-agnostic (Wave 10), but parity tests against upstream still show bf16-noise residual drift in fp32 runs because upstream's intermediate tensors are bf16. Full fp32-clean parity would require patching the local upstream clone or adding a dtype-config flag upstream. Affects only fp32 parity testing, not production. | TRACKED FOLLOW-UP (LOW PRIORITY) |