Skip to content

Commit 850325c

Browse files
committed
[skills] Add FLUX-port learnings to add-model + seed-ssim skills
Three concrete, generalizable improvements derived from PR hao-ai-lab#1321 (FLUX.1-dev port fixes) that the next model-port will hit: 1. add-model-02-parity — Tolerance guide stopped at "Full DiT, cross-kernel bf16: 0.1". FLUX (57 layers) needed atol=0.5 — observed max=0.5, mean=0.04, median=0 on A40. Added a "Very deep DiT (50+ layers), bf16" row and a "Calibrating atol > 0.1" subsection that requires diagnostic prints (max/mean/median/p99) so reviewers can verify the calibration without rerunning. Also distinguishes the healthy bf16-tail signature (median≈0, mean<<atol) from a real bug signature (mean_diff>>0.1). 2. add-model-09-pipeline — Step 4 listed two surfaces (sampling_param.py + CLI args) for adding new generation kwargs. The current architecture has four: sampling_param.py, api/schema.py SamplingConfig, the schema_parity_inventory YAML (moved + expected_dests), and the test_parser.py roundtrip dict snapshot. Missing any one fails CI with a different error; we hit three separate failures during PR hao-ai-lab#1321 before getting it green. Expanded Step 4 to enumerate all four with the failure mode for each. 3. seed-ssim-references — Skill documented .mp4 and .pt artefacts. T2I tests that reuse run_text_to_video_similarity_test produce .png when num_frames=1; reference_videos_cli.py copy-local silently skips PNG (walks .mp4/.pt only, reports "0 copied files"). Added .png as a third artefact type and a step-5 gotcha with the manual cp workaround plus the .gitignore negation pitfall. Each change is a small in-place edit to an existing skill file. No new skills added, no skill removed. Verifiable against the FLUX port: every failure mode called out here has a corresponding commit in PR hao-ai-lab#1321 (c82dd31 for atol, d1a63c8 for schema parity inventory, aabf179 for test_parser snapshot, 5b5fdcf for SSIM PNG copy).
1 parent 3668279 commit 850325c

3 files changed

Lines changed: 66 additions & 4 deletions

File tree

.agents/skills/add-model-02-parity/SKILL.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,7 @@ Tolerance guide:
191191
| Single block, same kernel | `1e-4` / `1e-4` | Tight default. |
192192
| Full DiT, aligned kernels | `1e-2` / `1e-2` | Cross-layer accumulation. |
193193
| Full DiT, cross-kernel bf16 | `0.1` / `0.1` | Also require abs-mean drift below 5% and per-modality diagnostics. |
194+
| Very deep DiT (50+ layers), bf16 | `0.5` / `0.0` | Tail errors from per-GEMM bf16 epsilon (~7.8e-3) accumulated across all layers. Must justify via diagnostic prints — see below. |
194195
| VAE decode fp32 | `5e-2` / `5e-2` | After normalization alignment. |
195196
| Encoder wrapper around same HF class | `1e-3` / `1e-3` | Should be near-zero. |
196197

@@ -203,6 +204,28 @@ hooks. Use `docs/contributing/activation_trace.md` to keep
203204
`FASTVIDEO_TRACE_LAYERS`, `FASTVIDEO_TRACE_STATS`, and `FASTVIDEO_TRACE_STEPS`
204205
identical across FastVideo and upstream traces.
205206

207+
### Calibrating atol > 0.1
208+
209+
When a deep DiT (e.g. FLUX with 57 transformer blocks) produces `max_diff > 0.1`
210+
under bf16, do not silently bump `atol` to make the test pass. Print four metrics
211+
and assert that the *distribution* — not just the max — looks healthy:
212+
213+
```python
214+
abs_diff = (hf_out.float().cpu() - fv_out.float().cpu()).abs()
215+
print(f"max_diff={abs_diff.max():.4f} mean_diff={abs_diff.mean():.4f} "
216+
f"median_diff={abs_diff.median():.4f} "
217+
f"p99_diff={abs_diff.flatten().kthvalue(int(0.99 * abs_diff.numel())).values:.4f}")
218+
```
219+
220+
Healthy bf16-tail signature (FLUX, 57 layers, observed on A40):
221+
`max=0.5, mean=0.04, median=0, p99=0.25`. Median near zero and mean ≪ atol
222+
prove the bulk of elements match — only the tail diverges due to accumulation.
223+
224+
Real bug signature: `mean_diff >> 0.1` or `median_diff > 0.01`. Wrong weights,
225+
swapped layers, or missing residuals push the mean up, not just the max. Keep
226+
the diagnostic print in the committed test so reviewers can verify the
227+
calibration without rerunning.
228+
206229
Useful local commands:
207230

208231
```bash

.agents/skills/add-model-09-pipeline/SKILL.md

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,28 @@ Use this mode first.
6969
Runtime pipeline resolution is exact: `model_index.json["_class_name"]` must
7070
match a registered `EntryClass.__name__`, or a wrapper/alias class in
7171
`EntryClass`. Registry detectors do not select the executable pipeline class.
72-
4. Add new public generation kwargs to `fastvideo/api/sampling_param.py` before
73-
examples or presets use them. `SamplingParam.update()` ignores unknown keys
74-
except for logging, and preset defaults apply only to declared fields. Add CLI
75-
args when the option should be available from command-line entrypoints.
72+
4. Add new public generation kwargs across the **full schema-parity surface**
73+
before examples or presets use them. A missed surface fails CI in a different
74+
way each time, so touching all four in one commit prevents three follow-ups:
75+
- `fastvideo/api/sampling_param.py``SamplingParam.update()` ignores unknown
76+
keys except for logging, and preset defaults apply only to declared fields.
77+
Add CLI args here when the option should be available from command-line
78+
entrypoints (note: the inference CLI is now config-only — most new fields
79+
are reached via dotted overrides, not new flags).
80+
- `fastvideo/api/schema.py` — add the field to `SamplingConfig` with the same
81+
default. `test_inventory_targets_exist_in_typed_schema` walks
82+
`request.sampling.<field>` and will assert-fail if the dataclass is missing
83+
the attribute.
84+
- `docs/design/inference_schema_parity_inventory.yaml` — register the field
85+
under `surfaces.sampling_param_base.moved` with target
86+
`request.sampling.<field>`. If the field has a live CLI dest (rare under
87+
the config-only CLI), also add it to `cli.generate.expected_dests`
88+
alphabetically, otherwise `test_cli_dest_inventory_matches_live_parsers`
89+
fails.
90+
- `fastvideo/tests/api/test_parser.py`
91+
`test_load_run_config_supports_yaml_roundtrip` compares an exact hardcoded
92+
dict snapshot of every `SamplingConfig` field. Add the new field with its
93+
default value in declaration order, or the dict-equality assertion fails.
7694
5. Put loader-time changes in `load_modules()` or earlier, not
7795
`initialize_pipeline()`. `ComposedPipelineBase.__init__` loads modules before
7896
`post_init()` calls `initialize_pipeline()`, so process-global flags, loader

.agents/skills/seed-ssim-references/SKILL.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ side-by-side per `(model_id, backend, prompt)`:
1919
metadata + `slice_spec` + `format_version`) for tests that call
2020
`run_text_to_latent_similarity_test` in `latent_similarity_utils.py`.
2121
Compared via cosine distance on the slice and the full tensor.
22+
- **`.png`** — pixel ground-truth for **T2I** tests (`num_frames=1`) that
23+
reuse `run_text_to_video_similarity_test` but produce a single frame. The
24+
helper writes a `.png` instead of a `.mp4` when the output has no time
25+
dimension. Compared via SSIM the same way.
2226

2327
This skill:
2428

@@ -111,6 +115,23 @@ and 6 are artefact-type-agnostic — `_iter_reference_files`,
111115
`copy_generated_to_reference`, and `upload_reference_videos` already walk
112116
both `.mp4` and `.pt` (see `reference_videos_cli.py`).
113117

118+
**T2I gotcha:** when the helper produces a `.png` (T2I test with
119+
`num_frames=1`), `reference_videos_cli.py copy-local` currently walks
120+
`.mp4`/`.pt` only and reports `0 copied files` without erroring. After step 5,
121+
verify the destination contains the seeded file; if it's empty, copy the PNG
122+
manually:
123+
124+
```bash
125+
cp ./generated_videos_modal/default/generated_videos/L40S_reference_videos/<model_id>/<backend>/*.png \
126+
fastvideo/tests/ssim/reference_videos/default/L40S_reference_videos/<model_id>/<backend>/
127+
```
128+
129+
Then `git add -f` the PNG: the repo `.gitignore` has a broad `*.png` rule, and
130+
the `reference_videos/**` negation only applies to extensions explicitly
131+
re-allowed *after* the catch-all (see `.gitignore` near the
132+
`reference_videos/**` block — add `!fastvideo/tests/ssim/reference_videos/**/*.png`
133+
there if the negation is missing).
134+
114135
If either check fails, stop and tell the user what's wrong.
115136

116137
### 2. Run the test on Modal L40S

0 commit comments

Comments
 (0)