diff --git a/.agents/skills/add-model-02-parity/SKILL.md b/.agents/skills/add-model-02-parity/SKILL.md index 0ee95b97f5..5a96f34a56 100644 --- a/.agents/skills/add-model-02-parity/SKILL.md +++ b/.agents/skills/add-model-02-parity/SKILL.md @@ -197,6 +197,12 @@ Tolerance guide: Element-wise `assert_close` alone is not enough for deep full-DiT parity. Also log global abs-mean drift and per-modality summaries. +When a non-skip component parity run is numerically red after weight/input +checks, invoke `../add-model-08-trace/SKILL.md` before adding bespoke forward +hooks. Use `docs/contributing/activation_trace.md` to keep +`FASTVIDEO_TRACE_LAYERS`, `FASTVIDEO_TRACE_STATS`, and `FASTVIDEO_TRACE_STEPS` +identical across FastVideo and upstream traces. + Useful local commands: ```bash diff --git a/.agents/skills/add-model-03-port-dit/SKILL.md b/.agents/skills/add-model-03-port-dit/SKILL.md index 035cf9ea74..028f6b3466 100644 --- a/.agents/skills/add-model-03-port-dit/SKILL.md +++ b/.agents/skills/add-model-03-port-dit/SKILL.md @@ -94,8 +94,12 @@ Run the shared parity-debug loop. The component test command is: pytest -v -s ``` -For numerical drift, narrow the first divergent block with per-block hooks or -intermediate tensor comparisons before changing layers. +For numerical drift, use `../add-model-08-trace/SKILL.md` before writing bespoke +hooks. Start with FastVideo's activation trace (`fastvideo/hooks/activation_trace.py`; +`docs/contributing/activation_trace.md`) and a block-level regex such as +`FASTVIDEO_TRACE_LAYERS="^block\.layers\.[0-9]+$"`. Only fall back to custom +per-block hooks if the needed boundary or statistic is not exposed by +`FASTVIDEO_TRACE_STATS`. ## Escape Hatches diff --git a/.agents/skills/add-model-08-trace/SKILL.md b/.agents/skills/add-model-08-trace/SKILL.md index 65fd1e748b..fb3363b77b 100644 --- a/.agents/skills/add-model-08-trace/SKILL.md +++ b/.agents/skills/add-model-08-trace/SKILL.md @@ -1,6 +1,6 @@ --- name: add-model-08-trace -description: Use during /add-model Phase 6 when component parity has failed and root cause requires layer-by-layer divergence analysis. Instruments both the official reference and FastVideo port with forward hooks to find the first numerical divergence point. +description: Use during /add-model Phase 6 when component parity has failed and root cause requires layer-by-layer divergence analysis. Uses FastVideo activation trace first, falling back to custom hooks only for boundaries or stats the utility cannot observe. --- # Add-Model Trace @@ -38,104 +38,90 @@ Required inputs before starting: - Shared deterministic test inputs (same tensors on both sides). - The component parity test file path and its current failure output. -## Hard Rules: Instrumentation Hierarchy +## Primary Path: FastVideo Activation Trace -Apply these in priority order. Use the highest-priority method that works for -the target site. +Use FastVideo's first-class activation trace before writing custom hooks: +`fastvideo/hooks/activation_trace.py`, documented in +`docs/contributing/activation_trace.md`. -### (1) Forward hooks (PREFERRED) +Pipeline runs attach trace to the transformer during pipeline initialization. +Component-only parity harnesses may call `attach_activation_trace(model)` from +local test/debug code; do not add trace calls to production model code. -`module.register_forward_hook(...)` and `register_forward_pre_hook(...)`. -Always within `try/finally` with `handle.remove()`. Zero source residue. +Prefix the failing parity command with a tight layer regex: -```python -handle = module.register_forward_hook(fn) -try: - output = model(inputs) -finally: - handle.remove() +```bash +FASTVIDEO_TRACE_ACTIVATIONS=1 \ +FASTVIDEO_TRACE_LAYERS="^block\.layers\.[0-9]+$" \ +FASTVIDEO_TRACE_STATS="abs_mean,sum,max,shape" \ +FASTVIDEO_TRACE_STEPS="0" \ +FASTVIDEO_TRACE_OUTPUT="/tmp/opencode/fv_trace.jsonl" \ +pytest tests/local_tests -k "parity" -v -s ``` -### (2) Runtime monkey-patch (PREFERRED over source edits) +Match the layer regex to the actual `model.named_modules()` names. Empty or +broad regexes are expensive; prefer block-level names first, then narrow to +submodules after the first divergent block is known. -`module.attr = wrapped_func` or `cls.method = wrapped_method`, restored via -`try/finally` (save original first). Use for free functions and non-Module -sites such as activation functions (`swiglu`, `apply_rotary_emb`) that cannot -be hooked as `nn.Module` submodules. - -```python -original = cls.method -cls.method = wrapped -try: - output = model(inputs) -finally: - cls.method = original -``` +## Trace Compare Contract -### (3) Source edits in FastVideo's own code +One JSONL file per side. FastVideo output should use `FASTVIDEO_TRACE_OUTPUT`; +the upstream harness should emit the same JSONL shape: -Only when (1) and (2) are insufficient. Track all edits within a single named -`git stash` boundary OR a temporary branch. Run `git diff` before closing the -investigation to confirm the stash or branch is clean. The cleanup gate -enforces this. +```json +{"module":"block.layers.0","tensor":"out","step":0,"abs_mean":0.0123,"sum":1.0,"max":0.5,"shape":[1,16,32]} +``` -### (4) Source edits in official repo source +Compare rows by `(module, step, tensor)`. The first row whose `shape`, +`abs_mean`, or `max` diverges beyond the component tolerance is the first broken +boundary. Keep `FASTVIDEO_TRACE_LAYERS`, `FASTVIDEO_TRACE_STATS`, and +`FASTVIDEO_TRACE_STEPS` identical between sides; if row order differs, sort or +normalize before diffing. -Allowed if EITHER: +## Drill-Down Loop -- (a) The official repo is a git-tracked clone (e.g. `daVinci-MagiHuman/` at - the repo root): use `git diff` in the clone path to verify cleanup. -- (b) It's installed editable (`pip install -e .`): use `git diff` in the - editable source path to verify cleanup. +**Initial run:** trace every top-level block (`^block\.layers\.[0-9]+$` or the +family's equivalent). Identify the first block index where `abs_mean` or `max` +drifts beyond tolerance while earlier blocks match. -If the official repo is installed non-editable in site-packages: back up the -target file (`cp original.py original.py.trace-backup`) before editing, then -restore from backup at the end (or `pip install --force-reinstall `). -The cleanup gate verifies via diff-against-backup or zero-diff-in-clone. +**Drill run:** tighten `FASTVIDEO_TRACE_LAYERS` to submodules inside the first +divergent block: attention output, MLP projections, norm outputs, modality +adapters, or other named boundaries exposed by `named_modules()`. -## Logging Contract +**Iterate:** if the first divergent operation is a free function or tensor op not +visible as an `nn.Module`, use the fallback instrumentation hierarchy below. -One log file per side. Paths: +The loop ends when the first divergent submodule or operation is identified with +a file:line citation in the official source. -``` -/tmp/opencode/__up_layers.log -/tmp/opencode/__fv_layers.log -``` +## Fallback Instrumentation Hierarchy -Format: one line per captured tensor, space-separated: +Use these only when activation trace cannot observe the needed boundary or +statistic. -``` - -``` - -Example: +### (1) Custom forward hooks -``` -block[00] (1,512,1024) 0.012345 6.3210 -0.4321 0.4321 -``` +`module.register_forward_hook(...)` and `register_forward_pre_hook(...)`. +Always within `try/finally` with `handle.remove()`. Zero source residue. -Keep the format diff-friendly. Running `diff /tmp/opencode/x_up.log -/tmp/opencode/x_fv.log` should highlight the first divergent line directly. -Retain side-by-side stdout output alongside the per-side files for human -review. +### (2) Runtime monkey-patch -## Drill-Down Loop +`module.attr = wrapped_func` or `cls.method = wrapped_method`, restored via +`try/finally` (save original first). Use for free functions and non-Module sites +such as activation functions (`swiglu`, `apply_rotary_emb`). -**Initial run:** attach hooks to every top-level block (`model.block.layers[i]` -or equivalent). Identify the first block index `NN` where abs_mean relative -drift exceeds 0.5% compared to the previous block. +### (3) Source edits in FastVideo's own code -**Drill run:** set `_DEBUG_DRILL_LAYER=NN` and re-run. The script -attaches submodule hooks inside block `NN`: attention output, mlp.pre_norm, -mlp.up_gate_proj, mlp.down_proj input (via pre-hook) and output, mlp output, -attn_post_norm (if present), mlp_post_norm (if present). +Only when (1) and (2) are insufficient. Track all edits within a single named +`git stash` boundary OR a temporary branch. Run `git diff` before closing the +investigation to confirm cleanup. -**Iterate:** if the drill run points to a free function (e.g. an activation -not wrapped in an `nn.Module`), switch to a monkey-patch (method 2) to -intercept its output via the next module's pre-hook. +### (4) Source edits in official repo source -The loop ends when the first divergent submodule is identified with a -file:line citation in the official source. +Allowed only when hook and monkey-patch approaches cannot capture the site. +For git-tracked or editable official clones, use `git diff` in the clone path to +verify cleanup. For non-editable site-packages, back up the target file before +editing and restore it before handoff. ## Hypothesis Toggles @@ -194,11 +180,13 @@ Escalate to the calling bucket skill when: Return to the calling subagent with: -- File paths to per-side logs (`/tmp/opencode/__{up,fv}_layers.log`). -- The identified first divergent layer or submodule name. +- FastVideo trace JSONL path and upstream trace JSONL path. +- Trace settings used: `FASTVIDEO_TRACE_LAYERS`, `FASTVIDEO_TRACE_STATS`, and + `FASTVIDEO_TRACE_STEPS`. +- The first divergent `(module, step, tensor)` row and observed drift. - The upstream file:line citation where the divergence originates. -- Hypothesis verdict if an A/B toggle was used (e.g. "PATCH_LINEAR=1 closes - the gap, confirming dtype-cast difference in PackedExpertLinear"). +- Fallback hook/patch verdict if activation trace could not observe the boundary. +- Hypothesis verdict if an A/B toggle was used, for example `PATCH_LINEAR=1`. - Cleanup-gate status: `[cleanup-gate] PASS` or a list of unresolved items. The calling agent uses this to scope the production fix in the FastVideo @@ -206,10 +194,14 @@ component file. ## References -- `templates/block_trace_debug.py` in this skill directory: the canonical - template this skill generalizes. +- `docs/contributing/activation_trace.md` for canonical activation-trace env vars, + JSONL output, cost model, and troubleshooting. +- `fastvideo/hooks/activation_trace.py` for the implementation and + `attach_activation_trace(model)` entry point. +- `templates/block_trace_debug.py` in this skill directory: fallback custom-hook + template when activation trace cannot observe the needed boundary or stat. - `tests/local_tests/transformers/_debug_magi_human_block_parity.py` in the - FastVideo3 repo: the worked magi-human example this skill was extracted from. + FastVideo3 repo: historical worked example for custom hook/patch debugging. - `add-model/SKILL.md` Phase 6: the calling context for this skill. - `add-model-03-port-dit/SKILL.md`, `add-model-04-port-vae/SKILL.md`, `add-model-05-port-encoder/SKILL.md`, `add-model-06-port-generic/SKILL.md`: diff --git a/.agents/skills/add-model-09-pipeline/SKILL.md b/.agents/skills/add-model-09-pipeline/SKILL.md index 0c4dcaabbd..e91257b0a6 100644 --- a/.agents/skills/add-model-09-pipeline/SKILL.md +++ b/.agents/skills/add-model-09-pipeline/SKILL.md @@ -157,6 +157,13 @@ Debug pipeline drift in this order: channel order, sample rate, FPS, and final slicing. 7. Add targeted stage-level diagnostics to identify the first divergent stage. +If stage diagnostics show the first bad stage is transformer/denoising or a +mid-DiT block, enable activation trace before adding ad hoc pipeline prints; see +`docs/contributing/activation_trace.md` and `../add-model-08-trace/SKILL.md`. +Keep `FASTVIDEO_TRACE_LAYERS`, `FASTVIDEO_TRACE_STATS`, and +`FASTVIDEO_TRACE_STEPS` identical across reruns so pipeline parity traces diff +one-to-one. + If the first divergence belongs to component implementation, strict loading, or conversion mapping, stop pipeline edits and return `next_step=return_to_phase_6` with the exact failing evidence. Do not patch conversion from this skill. diff --git a/.agents/skills/add-model/SKILL.md b/.agents/skills/add-model/SKILL.md index f88cdd47e4..a5975c3c47 100644 --- a/.agents/skills/add-model/SKILL.md +++ b/.agents/skills/add-model/SKILL.md @@ -267,6 +267,11 @@ conversion, route it through `../add-model-07-conversion/SKILL.md` with a retry request matching `contracts/conversion_request.md`, then resume the component skill with the updated conversion handoff. +When a component failure narrows to layer-by-layer numerical drift, load +`../add-model-08-trace/SKILL.md` before writing custom hooks. It uses +`fastvideo/hooks/activation_trace.py`; canonical env vars and JSONL format are +documented in `docs/contributing/activation_trace.md`. + Phase 6 ends only when every required component handoff reports `parity_status=non_skip_pass`, or when a precise blocker or escape hatch is recorded in `port_state_file`.