Skip to content

Commit 9486018

Browse files
SolitaryThinkerSolitaryThinker
committed
[docs]: MagiHuman provenance - AGENTS.md, JOURNAL.md, lessons
Authored navigation/provenance artefacts for the MagiHuman pipeline package: - fastvideo/pipelines/basic/magi_human/AGENTS.md: 6-section package manifest (file table, parity invariants, cross-refs, run book, open questions, provenance). Provenance section will be finalized in 8/8 with the actual conversion + activate PR numbers. - fastvideo/pipelines/basic/magi_human/JOURNAL.md: 14-wave port-state journal brought over from tests/local_tests/magi-human.md to live alongside the pipeline package (its canonical home). - .agents/lessons/2026-05-07_silent-channel-major-packing-bugs.md: lesson extracted from the channel-major einops bug hunt (Wave 14, commit 6d19069). - .agents/lessons/2026-05-07_dit-dtype-boundary-with-flash-attn.md: lesson extracted from the four-fix DiT dtype boundary alignment (commit 3a4816c). - fastvideo/pipelines/AGENTS.md: new optional 'Per-Package AGENTS.md and JOURNAL.md' section documenting this convention so future ports inherit the pattern. Cites magi_human as the canonical example. Note: the third lesson (conversion-cast-bf16-suffix-allowlist) lands in 7/8 alongside the conversion script. Magi-Stack: 6/8
1 parent 429e4ab commit 9486018

5 files changed

Lines changed: 981 additions & 0 deletions

File tree

‎.agents/lessons/2026-05-07_dit-dtype-boundary-with-flash-attn.md‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
---
2+
date: 2026-05-07
3+
experiment: PR #1280 (daVinci-MagiHuman port), DiT parity bring-up
4+
category: porting
5+
severity: important
6+
---
7+
8+
# DiT Dtype Boundary Alignment with Flash-Attn-Style Backends
9+
10+
## What Happened
11+
12+
DiT bit-exact parity for daVinci-MagiHuman against the upstream reference
13+
sat at `diff_max=0.5` after the architecture port was complete and weight
14+
loading was correct. The error grew with depth (later layers diverged more
15+
than earlier ones), suggesting an accumulating numerical drift rather than
16+
a structural mismatch. None of the obvious culprits (RoPE, GQA expansion,
17+
attention mask handling) accounted for the pattern.
18+
19+
## Root Cause
20+
21+
Four cumulative dtype-boundary mismatches, each individually small but
22+
together pushing parity from `diff_max=0.5` to bit-exact (`diff_max=0.0`):
23+
24+
1. **SDPA inputs were not cast to bf16.** Upstream's `flash_attn_with_cp`
25+
internally casts Q/K/V to bf16 at `dit_module.py:508` before the kernel.
26+
FastVideo was passing fp32 tensors through, getting numerically different
27+
intermediates even though the kernel accepts both.
28+
29+
2. **Post-attention output was kept in bf16 across the per-head gating
30+
multiply.** Upstream upcasts to fp32 before the gating, FastVideo did the
31+
gate in bf16 then upcast.
32+
33+
3. **A residual-stream cast at the block boundary.** FastVideo had a
34+
`.to(bf16)` then `.to(fp32)` at the start of each block. Upstream keeps
35+
the residual stream **continuously in fp32** across all 40 layers; only
36+
the inputs to specific kernels are temporarily downcast.
37+
38+
4. **Parity test scheduler used a double-shift.** A separate per-block fix
39+
(Wave 11 production migration) — single-shift schedule is what upstream
40+
uses; the parity test was double-shifting.
41+
42+
## Fix / Workaround
43+
44+
Four cumulative changes in `fastvideo/models/dits/magi_human.py` (commit
45+
`3a4816cb`), each with a comment at the call site explaining the upstream
46+
parity rationale:
47+
48+
- Cast SDPA inputs to bf16 right before the attention call.
49+
- Upcast attention output to fp32 before the per-head gate multiply.
50+
- Drop the residual-stream `.to(bf16)`/`.to(fp32)` wrapper at the block
51+
boundary; let the residual stay fp32 throughout.
52+
- Single-shift schedule in the parity test fixture (matches upstream Wave 11).
53+
54+
## Prevention
55+
56+
1. **For any DiT port with a flash-attn-style backend**, treat the dtype of
57+
the residual stream as a load-bearing invariant, not a performance knob.
58+
Document it in the model's per-pipeline AGENTS.md. MagiHuman's invariant:
59+
*residual stream stays fp32 across all blocks; only kernel inputs are
60+
temporarily bf16*.
61+
62+
2. **Use layer-by-layer activation hooks** when DiT parity is close-but-not-
63+
bit-exact and the gap grows with depth. The
64+
`fastvideo/hooks/activation_trace.py` infra exists exactly for this case
65+
(`add-model-trace` skill). In MagiHuman's case it would have localized the
66+
first divergence point in one pass.
67+
68+
3. **The `add-model-port-dit` skill** should explicitly call out:
69+
- SDPA input dtype must match the upstream kernel's internal cast.
70+
- Post-attention upcast happens **before** any per-head gate, not after.
71+
- Residual stream dtype across block boundaries is a parity invariant.
72+
These rules apply to any DiT port whose upstream uses a flash-attn-style
73+
backend (`flash_attn_with_cp`, `flex_flash_attn_func`, etc.).
74+
75+
4. **Add an "intermediate-layer parity" test** for new DiT ports — comparing
76+
activations at layer 5, 10, 20, 30 — not just the final output. A growing-
77+
with-depth pattern is otherwise indistinguishable from "almost right".

‎.agents/lessons/2026-05-07_silent-channel-major-packing-bugs.md‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
---
2+
date: 2026-05-07
3+
experiment: PR #1280 (daVinci-MagiHuman port), Wave 14
4+
category: porting
5+
severity: critical
6+
---
7+
8+
# Silent Channel-Major Token-Packing Bugs
9+
10+
## What Happened
11+
12+
While porting daVinci-MagiHuman (`fastvideo/pipelines/basic/magi_human/`),
13+
the pipeline-parity test passed bit-exactly but the E2E user-visible output
14+
was **pure static noise**. Latent tensors compared identically against the
15+
upstream reference at every checkpointed boundary, yet decoded videos showed
16+
no recognizable content. The discrepancy reproduced on every variant
17+
(base / distill / SR-540p / SR-1080p) with the same noise profile.
18+
19+
## Root Cause
20+
21+
Video tokens were being packed **spatial-major** instead of **channel-major**:
22+
23+
```python
24+
# What we had (spatial-major, WRONG)
25+
einops.rearrange(x, "b c (T pT) (H pH) (W pW) -> b (T H W) (pT pH pW C)", ...)
26+
27+
# What upstream's UnfoldNd produces (channel-major, CORRECT)
28+
einops.rearrange(x, "b c (T pT) (H pH) (W pW) -> b (T H W) (C pT pH pW)", ...)
29+
```
30+
31+
A single-character einops reorder. The pipeline-parity test used FastVideo's
32+
own packer on **both** sides of the comparison, so the bug was invisible there
33+
— both sides agreed on the wrong layout. The DiT consumed those tokens
34+
without complaint because the channel dimension only matters at decode time,
35+
when the VAE's first conv expects channel-major input. By that point the test
36+
boundary was already passed.
37+
38+
The bug was load-bearing for any token-packed format that downstream feeds
39+
into a `UnfoldNd`-shaped consumer. Wave 14 of the port took multiple bug-hunt
40+
iterations and an Oracle consultation to localize.
41+
42+
## Fix / Workaround
43+
44+
Single-character einops change in `stages/latent_preparation.py:_img2tokens`
45+
(commit `6d190693` of the original PR). After the fix, all four variants
46+
produced expected E2E output and the pipeline-parity tests still passed
47+
because both sides of the parity check are now correct.
48+
49+
## Prevention
50+
51+
1. **Never use the FastVideo-side packer on both sides of a parity test.**
52+
At least one parity boundary must compare against an upstream tensor
53+
produced by the upstream packer. For MagiHuman this means a separate
54+
`_img2tokens` parity test that feeds upstream `UnfoldNd` output as the
55+
reference, not FastVideo's reformatted equivalent.
56+
57+
2. **Add an E2E hash check** alongside latent-parity. The mp4 SHA was the
58+
first signal that something was wrong; if it had been part of the standard
59+
parity battery, the bug would have surfaced in Wave 1, not Wave 14. See
60+
`fastvideo/tests/ssim/test_magi_human_similarity.py` for the CI version.
61+
62+
3. **For any new model port that involves explicit tensor reshaping into
63+
tokens**, document the expected packing order (`(C pT pH pW)` vs
64+
`(pT pH pW C)`) at the call site and assert the layout matches the
65+
downstream consumer's expectation.
66+
67+
4. The `add-model-port-dit` skill's parity gate should require an E2E hash
68+
check for any DiT that does video token packing, not just latent
69+
bit-exactness.

‎fastvideo/pipelines/AGENTS.md‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,25 @@ Reuse `stages/text_encoding.py` if your model takes text → embeddings via a st
6464

6565
When forking, keep the file name model-prefixed (`longcat_*`, `gamecraft_*`) so the registry stays grep-able.
6666

67+
## Per-Package `AGENTS.md` and `JOURNAL.md` (optional)
68+
69+
Pipelines with non-trivial parity invariants, lazy-loaded shared components,
70+
or cross-component coordination MAY ship a per-package `AGENTS.md` and
71+
`JOURNAL.md` next to the pipeline files. The canonical example is
72+
`basic/magi_human/AGENTS.md` (umbrella HF repo, four lazy-loaded shared
73+
components, channel-major packing + dtype-boundary invariants).
74+
75+
When present:
76+
77+
- `basic/<model>/AGENTS.md` — manifest table of every file in scope, parity
78+
invariants, "if you change X re-run Y" cross-refs, run book, open
79+
questions, provenance (PR numbers + source SHAs).
80+
- `basic/<model>/JOURNAL.md` — port-state log for the original port; useful
81+
for future maintainers to understand why specific decisions were made.
82+
83+
These files are **not** required for simple ports that share stages and have
84+
no special parity invariants.
85+
6786
## Anti-Patterns
6887

6988
- Putting a full pipeline in a single file under `basic/<model>/` instead of composing stages.

‎fastvideo/pipelines/basic/magi_human/AGENTS.md‎

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
# `fastvideo/pipelines/basic/magi_human/` — daVinci-MagiHuman
2+
3+
**Generated:** 2026-05-07
4+
5+
Single-stream joint audio-visual generative pipeline. 4 variants × 2 input modes
6+
(T2V / TI2V) = 8 entrypoints. The DiT denoises video and audio latents in one
7+
unified token sequence — no cross-attention, channel-major token packing.
8+
9+
If you are touching this pipeline, **read the parity invariants and cross-refs
10+
sections below before editing any file in the manifest**.
11+
12+
## Manifest
13+
14+
| File | Role |
15+
|------|------|
16+
| `magi_human_pipeline.py` | Composed pipeline class. `load_modules` lazy-loads four shared upstream components (Wan 2.2 VAE, T5-Gemma, Stable Audio VAE, scheduler). |
17+
| `pipeline_configs.py` | Per-variant `PipelineConfig` dataclasses (`base`, `distill`, `sr_540p`, `sr_1080p`). |
18+
| `presets.py` | Preset registry per variant — entry point that `fastvideo/registry.py` imports. |
19+
| `__init__.py` | SPDX header only; no public exports beyond what `presets.py` registers. |
20+
| `stages/audio_decoding.py` | Decodes audio latents through the lazy Stable Audio VAE wrapper. |
21+
| `stages/denoising.py` | Joint AV denoising loop (32-step FlowUniPC with CFG=2 base, 8-step CFG=1 distill). |
22+
| `stages/latent_preparation.py` | Channel-major video token packing + audio interleave + reference-image masking. **Carries the channel-major packing invariant — see Parity Invariants.** |
23+
| `stages/reference_image.py` | TI2V reference-image conditioning. |
24+
| `stages/sr_denoising.py` | SR DiT denoising loop with cfg-trick guidance tensor. SR-1080p uses block-sparse video→video local-window attention on 32 of 40 SR DiT layers via a 3-block SDPA accumulator. |
25+
| `stages/sr_latent_preparation.py` | Trilinear-up of base latent + ZeroSNR noise + audio mix for the SR pass. |
26+
| `stages/__init__.py` | Re-exports stage classes. |
27+
| `JOURNAL.md` | 14-wave port-state journal; root cause writeups for the parity invariants below. |
28+
| `AGENTS.md` | This file. |
29+
30+
External coordinates of related files (read these too if you change wiring):
31+
32+
| Path | Role |
33+
|------|------|
34+
| `fastvideo/models/dits/magi_human.py` | DiT architecture port. |
35+
| `fastvideo/configs/models/dits/magi_human.py` | `MagiHumanVideoConfig` arch dataclass. |
36+
| `fastvideo/models/encoders/t5gemma.py` | T5-Gemma 9B UL2 text encoder port. |
37+
| `fastvideo/configs/models/encoders/t5gemma.py` | `T5GemmaEncoderConfig`. |
38+
| `fastvideo/models/vaes/sa_audio.py` | Lazy `OobleckVAE` wrapper (shared with `pipelines/basic/stable_audio/`). **Not new — pre-existed on main.** |
39+
| `fastvideo/models/vaes/oobleck.py` | `OobleckVAE` itself (pre-existing). |
40+
| `fastvideo/models/loader/component_loader.py` | `sr_transformer` module type alias added here. |
41+
| `scripts/checkpoint_conversion/convert_magi_human_to_diffusers.py` | Reference → FastVideo state-dict converter. **Carries the `_FP32_KEEP_SUFFIXES` invariant — see Parity Invariants.** |
42+
| `scripts/checkpoint_conversion/push_magi_human_to_hf.py` | Push converted weights to umbrella HF repo. |
43+
| `tests/local_tests/magi_human/` | 14-test parity battery (skipped in CI; GPU-gated). |
44+
| `tests/local_tests/helpers/magi_human_upstream.py` | Upstream daVinci-MagiHuman reference loader. |
45+
| `examples/inference/basic/basic_magi_human*.py` | 8 user-facing example scripts (one per variant × mode). |
46+
| `fastvideo/tests/ssim/test_magi_human_similarity.py` | CI-eligible SSIM regression. |
47+
48+
## Parity Invariants
49+
50+
These are **load-bearing**. Each one was a multi-wave bug hunt in the original
51+
port. Breaking any of them re-introduces a known production-only regression.
52+
See the matching lesson under `.agents/lessons/`.
53+
54+
### 1. Channel-major video token packing
55+
56+
Video tokens must be packed **`(C pT pH pW)`** — channel-major.
57+
58+
```python
59+
# CORRECT
60+
einops.rearrange(x, "b c (T pT) (H pH) (W pW) -> b (T H W) (C pT pH pW)", ...)
61+
# WRONG — produced pure noise in production E2E despite passing pipeline parity
62+
einops.rearrange(x, "b c (T pT) (H pH) (W pW) -> b (T H W) (pT pH pW C)", ...)
63+
```
64+
65+
Upstream `UnfoldNd` packs channel-major. Mismatching this passes the FastVideo
66+
self-parity test (both sides use FastVideo's packer) but breaks against the
67+
official reference, which only surfaces in production E2E. Lives in
68+
`stages/latent_preparation.py` — search for `_img2tokens`.
69+
70+
→ `.agents/lessons/2026-05-07_silent-channel-major-packing-bugs.md`
71+
72+
### 2. DiT dtype boundary discipline
73+
74+
Across DiT block boundaries, the residual stream **stays fp32**. SDPA inputs
75+
are cast to bf16 inside the attention call. Post-attention output is upcast to
76+
fp32 before the per-head gating multiply. **There is no block-boundary cast.**
77+
78+
These four rules are cumulative — relaxing any one re-introduces measurable
79+
parity drift (worst case `diff_max=0.5`, best case `diff_max≈1e-3` which is
80+
still not bit-exact).
81+
82+
→ `.agents/lessons/2026-05-07_dit-dtype-boundary-with-flash-attn.md`
83+
84+
### 3. Conversion `_FP32_KEEP_SUFFIXES` allowlist
85+
86+
`scripts/checkpoint_conversion/convert_magi_human_to_diffusers.py` runs with
87+
`--cast-bf16` by default. The allowlist of fp32-keep suffixes prevents a
88+
specific set of 8 tensors from being downcast. The base checkpoint and the
89+
FastVideo `final_linear`/adapter modules require these in fp32. The distill
90+
DiT was the canary: its parity went from `diff_mean=0.114` (silently wrong)
91+
to bit-exact when the allowlist was fixed.
92+
93+
If you add or rename DiT modules, check that any fp32-required tensors are
94+
covered by `_FP32_KEEP_SUFFIXES` and re-run `test_magi_human_distill_parity`.
95+
96+
→ `.agents/lessons/2026-05-07_conversion-cast-bf16-suffix-allowlist.md` (lands in 7/8)
97+
98+
### 4. Umbrella HF repo layout
99+
100+
User code is a single string per variant:
101+
102+
```python
103+
VideoGenerator.from_pretrained("FastVideo/MagiHuman-Diffusers/base")
104+
```
105+
106+
`fastvideo/utils.py:maybe_download_model` recognises the 3-segment
107+
`org/repo/subfolder` form and downloads only that subtree (not the full
108+
~75 GB repo). The umbrella repo:
109+
110+
```
111+
FastVideo/MagiHuman-Diffusers/
112+
├── base/{model_index.json, transformer/, scheduler/}
113+
├── distill/{model_index.json, transformer/, scheduler/}
114+
├── sr_540p/{model_index.json, transformer/, sr_transformer/, scheduler/}
115+
└── sr_1080p/{model_index.json, transformer/, sr_transformer/, scheduler/}
116+
```
117+
118+
Note: `vae/`, `text_encoder/`, `audio_vae/` subfolders are **deliberately
119+
absent**. Those four shared components are lazy-loaded by
120+
`MagiHumanPipeline.load_modules` from their canonical upstream repos
121+
(`Wan-AI/Wan2.2-TI2V-5B`, `google/t5gemma-9b-9b-ul2`,
122+
`stabilityai/stable-audio-open-1.0`). This relies on
123+
`fastvideo/utils.py:verify_model_config_and_directory` honoring
124+
`model_index.json` declarations.
125+
126+
## Cross-Refs (If you change X, re-run Y)
127+
128+
| If you touch... | Re-run at minimum |
129+
|---|---|
130+
| `stages/latent_preparation.py` (any token packing) | `test_magi_human_pipeline_parity` (base T2V) **and** one of the SR-540p/1080p tests **and** `examples/inference/basic/basic_magi_human.py` (E2E mp4 hash) |
131+
| `stages/denoising.py` or `stages/sr_denoising.py` | All four pipeline-parity tests (`{base, ti2v, sr540p, sr1080p}_pipeline_parity`) |
132+
| `fastvideo/models/dits/magi_human.py` (any layer) | `test_magi_human_parity` and `test_magi_human_distill_parity` (DiT-level) **before** the pipeline tests |
133+
| Anything dtype-related in the DiT | All DiT parity tests + verify the residual-stream dtype invariant manually with a layer-by-layer hook trace (see `fastvideo/hooks/activation_trace.py`) |
134+
| `scripts/checkpoint_conversion/convert_magi_human_to_diffusers.py` | Convert the distill checkpoint and run `test_magi_human_distill_parity` (it's the canary for `_FP32_KEEP_SUFFIXES`) |
135+
| `fastvideo/utils.py:maybe_download_model` (3-segment detector) | Smoke-load every existing 2-segment HF id in `registry.py` plus one MagiHuman variant |
136+
| `magi_human_pipeline.py:load_modules` | `test_magi_human_pipeline_smoke` + one full E2E `examples/inference/basic/basic_magi_human.py` |
137+
| `presets.py` or `pipeline_configs.py` | Both DiT parity tests (preset wiring leaks into module construction) |
138+
139+
## Run Book
140+
141+
```bash
142+
# Setup once
143+
export HF_TOKEN=hf_... # any of HF_TOKEN / HUGGINGFACE_HUB_TOKEN / HF_API_KEY works
144+
# Accept terms at:
145+
# - https://huggingface.co/GAIR/daVinci-MagiHuman
146+
# - https://huggingface.co/google/t5gemma-9b-9b-ul2
147+
# - https://huggingface.co/stabilityai/stable-audio-open-1.0
148+
# - https://huggingface.co/Wan-AI/Wan2.2-TI2V-5B
149+
150+
# Full parity battery (~233s on a single H100, GPU-gated)
151+
pytest tests/local_tests/magi_human/ -v -s
152+
153+
# E2E mp4 generation (any variant)
154+
python examples/inference/basic/basic_magi_human.py
155+
156+
# CI-eligible SSIM regression
157+
pytest fastvideo/tests/ssim/test_magi_human_similarity.py -v -s
158+
```
159+
160+
The base T2V mp4 hash should be `dcf5f2bf6534c7c0d91e7353e42b23db` — stable
161+
across all 43 commits of the original port.
162+
163+
## Open Questions
164+
165+
- **OQ-7** — Wan VAE decode shows a max diff of ~8e-4 against the diffusers
166+
reference (not bit-exact). Root cause is a known fp32 op-order drift in the
167+
Wan 2.2 VAE itself, not MagiHuman wiring. Tracked in `JOURNAL.md`. Documented
168+
here so the test author knows the tolerance is intentional, not a regression.
169+
170+
## Provenance
171+
172+
This pipeline was decomposed from a single 9,812-line PR
173+
([#1280](https://github.com/hao-ai-lab/FastVideo/pull/1280),
174+
`will/magi` @ `4e1603634d27c8e1b5c4cc5d9387f046547f5c49`) into a stack of
175+
focused PRs:
176+
177+
| Step | PR | Branch | Scope |
178+
|---|---|---|---|
179+
| Prereq A | [#1293](https://github.com/hao-ai-lab/FastVideo/pull/1293) | `will/activation-trace` | Generic activation-tracing infra |
180+
| Prereq B | [#1294](https://github.com/hao-ai-lab/FastVideo/pull/1294) | `will/loader-infra` | Loader umbrella-repo + optional component dirs |
181+
| 1/8 | [#1295](https://github.com/hao-ai-lab/FastVideo/pull/1295) | `will/magi-01-housekeeping` | gitignore, codespell, skills index |
182+
| 2/8 | [#1296](https://github.com/hao-ai-lab/FastVideo/pull/1296) | `will/magi-02-t5gemma` | T5-Gemma encoder + parity test |
183+
| 3/8 | [#1297](https://github.com/hao-ai-lab/FastVideo/pull/1297) | `will/magi-03-dit` | DiT + parity tests |
184+
| 4/8 | [#1298](https://github.com/hao-ai-lab/FastVideo/pull/1298) | `will/magi-04a-stages` | Pipeline stages + sr_transformer alias |
185+
| 5/8 | [#1299](https://github.com/hao-ai-lab/FastVideo/pull/1299) | `will/magi-04b-orchestrator` | Pipeline orchestrator + parity battery |
186+
| 6/8 | this PR | `will/magi-04c-provenance` | This AGENTS.md, JOURNAL.md, lessons, parent AGENTS.md hook |
187+
| 7/8 | (next) | `will/magi-05-conversion` | Checkpoint conversion + 3rd lesson |
188+
| 8/8 | (last) | `will/magi-06-activate` | Registry + examples + SSIM + codebase-map |
189+
190+
Provenance section will be finalized in 8/8 with the actual PR numbers for 7/8 and 8/8.

0 commit comments

Comments
 (0)