|
| 1 | +--- |
| 2 | +date: 2026-05-07 |
| 3 | +experiment: PR #1280 (daVinci-MagiHuman port), distill DiT parity bring-up |
| 4 | +category: porting |
| 5 | +severity: important |
| 6 | +--- |
| 7 | + |
| 8 | +# Conversion `--cast-bf16` Needs an FP32-Keep Suffix Allowlist |
| 9 | + |
| 10 | +## What Happened |
| 11 | + |
| 12 | +`scripts/checkpoint_conversion/convert_magi_human_to_diffusers.py --cast-bf16` |
| 13 | +produced a converted distill DiT checkpoint that loaded cleanly, ran end-to- |
| 14 | +end, and emitted reasonable output — but `test_magi_human_distill_parity` |
| 15 | +showed `diff_mean=0.114` against the upstream reference. The base DiT was |
| 16 | +bit-exact with the same conversion script. Only the distill variant |
| 17 | +regressed. |
| 18 | + |
| 19 | +The error was small enough that visual quality looked normal, but large |
| 20 | +enough to fail bit-exact parity. The MagiHuman base + distill DiTs share |
| 21 | +most of their architecture, so a difference that affected only distill was |
| 22 | +counterintuitive. |
| 23 | + |
| 24 | +## Root Cause |
| 25 | + |
| 26 | +`--cast-bf16` was downcasting **all** fp32 tensors to bf16 indiscriminately. |
| 27 | +The base checkpoint and the FastVideo `final_linear` / adapter modules |
| 28 | +require eight specific tensors to remain in fp32: |
| 29 | + |
| 30 | +- LayerNorm `gamma` / `beta` weights for the final residual exit |
| 31 | +- Adapter projection biases |
| 32 | +- A handful of scale parameters in the output projection chain |
| 33 | + |
| 34 | +These tensors participate in chains where bf16 precision causes accumulation |
| 35 | +error large enough to drift the parity check. The base DiT happened to not |
| 36 | +hit those specific chains in the path the test exercised (different |
| 37 | +attention mask shape, different audio interleave); the distill variant did. |
| 38 | + |
| 39 | +## Fix / Workaround |
| 40 | + |
| 41 | +Added `_FP32_KEEP_SUFFIXES` allowlist to |
| 42 | +`convert_magi_human_to_diffusers.py` (commit `829f70d3`) and gated `--cast- |
| 43 | +bf16` on it. Tensors whose state-dict key ends with any allowlisted suffix |
| 44 | +keep their original fp32 dtype regardless of the flag. |
| 45 | + |
| 46 | +Distill DiT parity went from `diff_mean=0.114` (silently wrong) to bit-exact |
| 47 | +in one commit. |
| 48 | + |
| 49 | +## Prevention |
| 50 | + |
| 51 | +1. **Treat `--cast-bf16` as opinionated, not blanket.** Any conversion |
| 52 | + script that supports a global dtype downcast flag MUST own an explicit |
| 53 | + allowlist of fp32-keep tensors, documented at the top of the file. |
| 54 | + |
| 55 | +2. **The `add-model-conversion` skill** should enforce two checks for any |
| 56 | + converter that ships a `--cast-bf16`-style flag: |
| 57 | + - Run the parity test for **every** variant of the model (base, distill, |
| 58 | + SR, etc.), not just the headline variant. Different variants exercise |
| 59 | + different code paths. |
| 60 | + - Diff the converted checkpoint's dtype map against the upstream |
| 61 | + reference and assert the allowlist covers every fp32 tensor in the |
| 62 | + reference. |
| 63 | + |
| 64 | +3. **For MagiHuman specifically**: if you add or rename DiT modules that |
| 65 | + touch `final_linear`, the adapter, or any LayerNorm in the residual exit |
| 66 | + path, **check that any fp32-required tensors are covered by |
| 67 | + `_FP32_KEEP_SUFFIXES`** in the conversion script and re-run |
| 68 | + `test_magi_human_distill_parity` (it's the canary). |
| 69 | + |
| 70 | +4. The lesson generalizes beyond MagiHuman: any DiT that uses bf16 mixed |
| 71 | + precision but keeps specific tensors in fp32 (a common pattern with |
| 72 | + flash-attn-style backends) needs this allowlist for any conversion that |
| 73 | + downcasts. |
0 commit comments