Skip to content

skills: correct the claims the sweep disproved, and cover what it found missing - #9

Merged
JadenFiotto-Kaufman merged 2 commits into
0.8from
fix/skills-sweep
Sep 8, 2026
Merged

JadenFiotto-Kaufman merged 2 commits into
0.8from
fix/skills-sweep

Conversation

@JadenFiotto-Kaufman

Copy link
Copy Markdown
Member

Implements item 24 of the stress-sweep decisions in full, plus the skill halves of item 23 points 2 and 3, plus two mid-task corrections from the coordinator.

What was broken

Claims inherited from the docs

  • "greedy by default" (nnsight/SKILL.md, references/generation.md ×2, references/api-reference.md, logit-lens/SKILL.md). generate calls HuggingFace's generate, which reads the checkpoint's generation_config — Qwen3-8B ships do_sample=True, temperature=0.6, top_p=0.95, Llama-3.2-1B-Instruct do_sample=True, temperature=0.6, top_p=0.9. This produced a logit lens disagreeing with its own generated tokens on 7/10 steps while the skill's wiring check passed. The lens example now passes do_sample=False and says a disagreeing last layer means sampling, not a broken lens.
  • vllm/SKILL.md point 2 carried docs/models/vllm.md:99's wrong residual: output[1] is the stream after this layer's attention, not the one entering the layer. The out[0] + out[1] recipe on the same page was already right and is unchanged.
  • "One condition, one forward pass" in diffusion-and-multimodal said a VLM sweep cannot be batched. True of a processor encoding, false of chat messages with an embedded image. Rewritten with a runnable three-layer sweep in one pass, and the vision-side scoping warning attached (item 7).
  • PEFT example adapted.model.layers[16].output[0]: adapted.model forwards to LlamaForCausalLM, which has no .layers, and a Llama block's .output is a bare tensor. Both now stated, path corrected to adapted.model.model.layers[16].output.
  • In-place SAE/probe attach (item 23.2) at sae-and-dictionary-learning/SKILL.md:92 and nnsight/references/modules-and-architectures.md:213, plus the two sibling examples in the same files so each teaches one form.

Claims the skills invented

  • causal-tracing's "roughly 7B parameters or more" (:23 and :382). Rewritten as a requirement on the model knowing the fact confidently and having depth to separate the sites, with two checks that can actually fail.
  • diffusion-and-multimodal: FLUX.2's read point (Qwen3 hidden states 9/18/27, so [-2] is a no-op — check the pipeline's encode_prompt); Qwen3-VL's adapter is not "(none)" (visual.deepstack_merger_list injects after LM layers 0-2, so zeroing only the merger leaves the answer unchanged); whisper's double encoder call; and a note that every diffusion example runs at guidance_scale=0.0, which is not the default.
  • model.scan(x) presented as generic in nnsight/SKILL.md and references/caching-and-scan.md. Verified: NNsight(torch.nn.Linear(4,4)).scan(...)AttributeError. scan comes from the meta/loadable mixin.
  • vllm/references/serving.md: -tp 2 prints two "Ignoring unknown argument" lines, not one; and n > 1 over serve= returns sequence 0's saves alone while result.outputs still shows all of them.

Coverage gaps — the largest part of the change.

  • envoys= appeared nowhere in the plugin and eproperty only as a pointer. modules-and-architectures.md gains a section with a runnable per-head .heads accessor attached to GPT-2's attention modules with envoys={GPT2Attention: Heads}, and the four things that bite (attribute fall-through, the transform's shape, the swallowed AttributeError, key="input").
  • The path reference gains MoE, Mamba/Mamba2, RWKV-4 and hybrid rows, an MoE sub-block table (router/experts/shared expert per family, and which families start dense), and the tensor-vs-tuple section is rewritten as "there is no rule that holds across families" with the Falcon-H1 1-tuple, RWKV-4 3-tuple and GPT-OSS (hidden, router_scores) cases. All measured with the updated inspector on transformers 5.15.
  • "Any PyTorch module" gains "Wrapping changes the module, and the change sticks" — finish the module (deepcopy/pickle/compile) before wrapping, do not replace a forward after wrapping, mutate the tree through the envoy. All three reproduced against 0.8.
  • .skip() added to the vLLM skill: why it can take the engine down rather than the request, that whether it does is the scheduler's choice, and that a vLLM decoder layer's .input is the positions tensor — so docs/usage/skip.md's pass-through idiom kills the engine. control-flow.md gains the rule that skip advances the run, so earlier reads must come above it.
  • A scope note on vllm/references/parallel-and-architectures.md's MoE section: its (logits, bias) router is vLLM's, and the same recipe is a no-op on TransformersModel.

scripts/inspect_model.py — a real code change, additive:

  • --task (a repo with no pipeline_tag, e.g. AntonV/mamba2-130m-hf, died on task inference) and --trust-remote-code (a custom architecture stopped at an interactive prompt).
  • Prints the resolved task beside the class it built, and says the task decides the class. On Qwen/Qwen3.5-0.8B the inferred image-text-to-text builds Qwen3_5ForConditionalGeneration with model.model.language_model.layers[i] and --task text-generation builds Qwen3_5ForCausalLM with model.model.layers[i] — the crash that cost one agent its first run.
  • Descends into MoE blocks: the key-paths list and the forward-order probe now name the router and the expert module, and pick the first non-dense layer when the model starts dense (NOTE: layer 0 is dense; the router first appears at [1]). The probe now prints the router's (logits, weights, index) tuple and the flat (B*T, D) expert rows directly.
  • Stops reporting every scan failure as "this pipeline branches on tensor data"; that diagnosis is now given only for GuardOnDataDependentSymNode.

Batching rule (coordinator mid-task correction)nnsight/references/batching.md gains "Which values are scoped to your rows" and "Forward keywords belong to the batch", stating the post-#722 rule: a leading dim that is the batch size or a whole multiple of it is scoped (which covers MoE (B*T, ...) and per-head (B*T*H, ...)), anything else goes to every invoke whole, and a write to one warns rather than failing silently. The MoE and VLM sections were written against the same rule.

What I tested

  • make test-local equivalent (pytest -q, ndif2): 150 passed, 2 failed. Both failures are pre-existing and environmental — vllm/SKILL.md and vllm/references/graph-taps.md fail with ModuleNotFoundError: No module named 'vllm'. Confirmed identical on a clean git archive HEAD checkout of the base commit.
  • tests/test_structure.py: 119 passed (links, frontmatter, symlinks, pre-0.8 API ban).
  • Ran directly, to make the claims measured rather than copied:
    • the causal-tracing re-run on Llama-3.2-1B: clean P(Paris) 0.9495, ' Tower' site 0.768 at layer 3, ' of' site 0.922 at layer 9 of 16 — reproduces the maintainer's numbers exactly.
    • generation_config sampling: gpt2 with do_sample=True gives 4 distinct continuations in 4 runs, 1 with do_sample=False.
    • the in-place-vs-replacement backward repro: inplace: RuntimeError ... modified by an inplace operation, replace: backward ok.
    • VLM chat-route batching on llava-interleave-qwen-0.5b-hf: two invokes batch; a three-layer ablation sweep is one pass; and with a two-image invoke beside a one-image one, multi_modal_projector.output is (3, 729, 1024) in both and zeroing it in the second moved the first's untouched logits by 8.4, while the LM-side control moved it by 0.0.
    • NNsight(nn.Linear).scanAttributeError; controller installed into the instance dict on first trace; forward replaced after wrapping → OutOfOrderError; net.append(...) after wrapping → torch 2 children, envoy 1, IndexError.
    • the new .heads eproperty block, and the inspector on gpt2, mamba-130m, mamba2-130m, RWKV-4, Falcon-H1, Qwen3-Next-MoE, Qwen3-MoE, GLM-4-MoE, DeepSeek-V3, GPT-OSS and Qwen3.5-0.8B.

Deliberately left alone, and two couplings

  • logit-lens's gemma-2 softcap handling is correct and verified. No change, as decided.
  • The tensor-parallel fused layout at plugins/nnsight/skills/vllm/references/parallel-and-architectures.md:45 — "A fused projection (qkv_proj, gate_up_proj) gathers in rank order — [q₀ k₀ v₀ | q₁ k₁ v₁] — so slice it by head, not by [:q_size]" — is correct for nnsight as it stands today, where docs/models/vllm-parallelism.md states it wrongly. Draft PR vllm: gather a fused projection into the layout tp=1 has nnsight#719 changes the gather so the doc becomes right and this line becomes wrong; it is unverified (nobody has a multi-GPU box to run it). If #719 lands, that line must change in the same commit.
  • The batching sections describe batching: scope token-flattened rows, and warn on writes that can't be scoped nnsight#722, which is not yet merged. They are written for the post-#722 behaviour, as instructed; the warning text is quoted from that branch's source. If #722 does not land, "a write that cannot be scoped warns" is the one sentence in this PR that is ahead of the code.
  • The CFG in-place claim is stated as a rule, not as a measurement. Item 6 records that an in-place edit is dropped under classifier-free guidance; I could not reproduce it in the single-implicit-invoke form the skill demonstrates (sd-turbo at guidance_scale=7.5: in-place and assignment both moved the image by 0.0494), and the explicit two-invoke form hit an unrelated StableDiffusionPipelineOutput error before I could measure it. So the text says the per-invoke value is stitched from two ranges and is a copy, recommends assignment, and does not claim a number I did not take. Worth revisiting when item 6 is picked up.
  • --peft for inspect_model.py was not added — out of scope for item 24. The model-editing-and-lora pointer that promised --grep lora would show an adapter tree is corrected to a named_modules() filter instead.
  • references/parallel-and-architectures.md's stale Qwen3.6 MoE tree and its (tensor, None) fused-projection outputs are in the consolidated report but not in item 24; left for whoever takes the vLLM doc items.

🤖 Generated with Claude Code

JadenFiotto-Kaufman and others added 2 commits September 4, 2026 14:23
nnsight 0.8 inverted which side of the collision moves. On a module whose own
child is named `output`, the child is now `.E_output` and `.output` keeps its
usual meaning; `.nns_output` is gone.

Where the old advice inverted along with the rule, it is replaced rather than
renamed. access-and-modify said to check `print(model)` for a `.output` that
looks like a module, which can no longer happen, so it points at the
`E_output/output` label the repr prints instead.

inspect_model.py built its note from the names it detected but printed a fixed
`.nns_output / .nns_input`; it now names the attributes it actually found.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nd missing

The stress sweep of nnsight 0.8 caught the plugin stating four things that are not
true and leaving out five that cost agents real time.

Wrong, and inherited from the docs: `generate` is not greedy by default — it uses
the checkpoint's `generation_config`, and Qwen3-8B and Llama-3.2-1B-Instruct both
sample from it; a vLLM decoder layer's `output[1]` is the residual after this
layer's attention, not the stream entering it; a VLM sweep *can* be batched over
the chat route, so it is one forward pass and not twelve; the PEFT example's
`adapted.model.layers[16].output[0]` is wrong twice over; and the in-place SAE
attach form writes the attachment's result into its own input, so backward through
it raises.

Wrong, and the skills' own: causal tracing does not need 7B parameters —
Llama-3.2-1B gives a textbook two-site trace on the skill's own prompt, so the
requirement is a confidently-known fact and enough depth, not a size; FLUX.2 reads
Qwen3 hidden states 9/18/27, so the `[-2]` recipe is a no-op there; Qwen3-VL's
adapter is not "(none)" and zeroing only the merger leaves the answer unchanged;
whisper runs its encoder twice; base `NNsight` has no `.scan()`; `-tp 2` prints two
"Ignoring unknown argument" lines, and `n > 1` over serve silently returns one
sequence's saves.

Missing: `envoys=` and `eproperty` appeared nowhere, so the path reference gains a
worked per-head accessor; the path table had no MoE, SSM or hybrid rows and stated
the tensor-vs-tuple rule as if it generalized; nothing said wrapping mutates the
module for the rest of the process; `.skip()` was absent from the vLLM skill
although it can take the engine down; and `skip` advancing the run past the module
was unstated.

`scripts/inspect_model.py` gains `--task` and `--trust-remote-code` (a repo with no
`pipeline_tag` died on task inference, a remote-code one on an interactive prompt),
prints the task beside the class it built, descends into MoE blocks — naming the
router and experts, and the first non-dense layer — and stops diagnosing every scan
failure as data-dependent control flow.

The batching sections state the rule from nnsight#722: a leading dim that is a whole
multiple of the batch size is scoped, and a write to a layout that cannot be scoped
warns.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@JadenFiotto-Kaufman
JadenFiotto-Kaufman merged commit 892aee0 into 0.8 Sep 8, 2026
1 of 2 checks passed
@JadenFiotto-Kaufman
JadenFiotto-Kaufman deleted the fix/skills-sweep branch September 8, 2026 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant