Repository navigation
fix: respect parallel MLP inputs in residual decomposition - #1946
Conversation
| ``resid_mid`` instead of ``resid_pre``, and with ``apply_ln=True`` the LN scale of | ||
| ln2 is used instead of ln1. The neurons stacked are unchanged. | ||
| ``resid_mid`` for sequential blocks or ``resid_pre`` for parallel blocks. | ||
| With ``apply_ln=True`` the LN scale of ln2 is used instead of ln1. |
There was a problem hiding this comment.
This is a bug that existed before this issue, but it is related, would you be willing to fix this here as well? Most parallel blocks have no ln2, because ln1 feeds both branches. So apply_ln=True with mlp_input=True still raises a KeyError for ln2.hook_scale here, in decompose_resid, and in get_full_resid_decomposition. Can we adjust the cached-scale lookup to resolve to the norm that feeds the MLP, so the normalized paths work on those blocks too?
There was a problem hiding this comment.
Thanks for catching this! I'll address it in this PR by resolving the cached scale from the norm that actually feeds the MLP, keeping independent ln2 norms where present and using ln1 for shared-norm parallel blocks. I'll also add coverage for the normalized decomposition paths on those shared-norm layouts.
There was a problem hiding this comment.
Fixed in 558597d. The cached-scale lookup now uses ln1 for shared-norm parallel blocks while preserving ln2 for blocks with an independent MLP norm. Missing cached ln2 scales still raise rather than falling back to ln1. I also added normalized decomposition coverage for the shared-norm layouts and a two-norm Falcon case.
|
Good solution here @emerardd, just one comment about a related bug that would be nice to fix along side it. If you don't have time to address that as well right now, let me know and I can merge this + file that as a separate issue for someone else to take on. |
jlarson4
left a comment
There was a problem hiding this comment.
Looks good! I'll merge once CI passes
|
Update: there is a bug with the Qwen3.5 testing model we use for CI that is preventing Full Code Coverage from passing, I will patch |
Description
Residual decomposition with
mlp_input=Trueassumes that the current layer's attention output feeds its MLP. That is correct for sequential blocks, but parallel blocks feed both branches fromresid_pre. On a tiny random NeoX with nonzero attention output biases, the existing component decomposition differs from the actualln2input by about 0.4; normalized attribution differs by about 23. Full head/neuron decomposition also includes the nonexistent current-layer attention contribution.This change declares the MLP-input topology on
BlockBridge/ParallelBlockBridgeand uses the target block's topology in the analysis paths:accumulated_bias()excludes the current attention output bias for parallel MLP inputs, while retaining all preceding contributions.resid_prerather than an absentresid_mid, including the fused normalized projection path.Model forward computation, hook naming, neuron selection and ordinary attention-input decomposition are unchanged. The existing method docstrings now describe parallel MLP-input semantics.
Type of change
Validation
The initial real-config NeoX regression had 16 failed / 16 passed before the fix: the parallel cases failed and the sequential controls passed. These models are built from config and do not download weights.
Expanded coverage checks layers 0/1, raw/normalized sums, batched/batchless caches, expanded/unexpanded neurons, vector/matrix projections, bias inclusion and remainder targets. Cross-architecture checks compare decomposition sums to captured normalization inputs, including shared-ln1 and separate-ln2 layouts.
mypy .: 389 source files passed.git diff --check: passed.The complete unit suite, pretrained checkpoints and GPU/MPS hardware were not tested.
Checklist