Skip to content

fix(bridge): remove dead _enable_ht_attention and its exclusive helpers - #1604

Merged
jlarson4 merged 1 commit into
TransformerLensOrg:dev-4.xfrom
sohv:fix/remove-dead-ht-attention
Aug 4, 2026
Merged

fix(bridge): remove dead _enable_ht_attention and its exclusive helpers#1604
jlarson4 merged 1 commit into
TransformerLensOrg:dev-4.xfrom
sohv:fix/remove-dead-ht-attention

Conversation

@sohv

@sohv sohv commented Aug 4, 2026

Copy link
Copy Markdown

Description

Follow-up to #1602, as agreed in #1601 - same defect pattern, same origin commit, separate PR.

ArchitectureAdapter._enable_ht_attention dispatched on three attention layouts, but two of the three branches called methods that do not exist anywhere in the tree:

Branch Attention layout Helper called Defined?
c_attn GPT-2 _extract_qkv_gpt2_style yes
q_proj / k_proj / v_proj Llama, Mistral, Qwen, Gemma, Phi, … _extract_linear_ht_format no
query_key_value GPT-NeoX, Falcon _extract_qkv_neox_style no

3efbd6e ("Cleanup (#1129)", 2025-11-15) deleted both helper definitions while keeping their call sites:

$ git show 3efbd6e -- transformer_lens/model_bridge/architecture_adapter.py | grep '^-.*def _extract'
-    def _extract_qkv_neox_style(self, query_key_value, n_heads, d_model, d_head):
-    def _extract_linear_ht_format(self, linear_module, n_heads, d_head, d_model):

Four # type: ignore[attr-defined] comments suppressed the resulting mypy errors, so CI stayed green. Only the GPT-2 branch could run; the other two raised AttributeError on entry.

No user was affected. The same commit also removed both callers (architecture_adapter.py:1249 and bridge.py:1144 as of 3efbd6e^), leaving the method unreachable — verified by searching *.py / *.ipynb / *.md and checking for dynamic getattr dispatch. The risk was latent: the method advertises itself as "architecture-agnostic" in its docstring, so the next person wiring it into component testing would have hit an immediate crash on every architecture except GPT-2.

What is removed

The whole unreachable cluster — 98 lines, the tail of the file:

  • _enable_ht_attention
  • _extract_qkv_gpt2_style, _extract_output_proj, _disable_hook_conversions — each called only from _enable_ht_attention, so they die with it. _disable_hook_conversions was already a no-op (pass, with a docstring saying it "currently does nothing in no_processing mode").
  • the four # type: ignore[attr-defined] comments
  • import einops, now unused (pruned by make format)

setup_component_testing and _wire_rotary_for_testing are untouched — they were never part of this path.

If you would rather keep the capability

The opposite fix is equally available: restore _extract_linear_ht_format and _extract_qkv_neox_style from 3efbd6e^ and re-wire a caller. That is a feature with its own design questions (which call path owns it, how it relates to setup_component_testing), so I have not assumed it — happy to send that version instead if you prefer.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have commented my code, particularly in hard-to-understand areas — this is a pure deletion; rationale is in the commit message and above
  • I have made corresponding changes to the documentation — no documented behaviour changes; the removed code was unreachable
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works — nothing to test: the code is unreachable and removed. Dropping the # type: ignore comments leaves the CI type-check job as the guard against reintroduction.
  • New and existing unit tests pass locally with my changes
  • I have not rewritten tests relating to key interfaces which would affect backward compatibility

_enable_ht_attention dispatched on three attention layouts, but two of the
three branches called methods that no longer exist: _extract_linear_ht_format
(split Q/K/V - Llama, Mistral, Qwen, Gemma) and _extract_qkv_neox_style
(fused query_key_value - GPT-NeoX, Falcon). 3efbd6e ("Cleanup (TransformerLensOrg#1129)")
deleted both definitions while keeping the call sites, so only the GPT-2
branch could run; the other two raised AttributeError on entry. Four
# type: ignore[attr-defined] comments kept mypy quiet about it.

The same commit removed both callers (architecture_adapter.py:1249 and
bridge.py:1144 as of 3efbd6e^), leaving the method unreachable - no user
impact, but it reads as a finished "architecture-agnostic" helper to anyone
wiring it into component testing.

Its three remaining helpers are called only from it, so they go too:
_extract_qkv_gpt2_style, _extract_output_proj, and _disable_hook_conversions
(whose body was already `pass`). einops becomes unused and is dropped.

setup_component_testing and _wire_rotary_for_testing are untouched.

Follow-up to TransformerLensOrg#1601/TransformerLensOrg#1602, which fixed the same pattern in the nanogpt
adapter; separate PR per maintainer request.
@jlarson4

jlarson4 commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Looks great, thank you @sohv!

@jlarson4
jlarson4 merged commit 2cec140 into TransformerLensOrg:dev-4.x Aug 4, 2026
25 checks passed
@sohv

sohv commented Aug 4, 2026

Copy link
Copy Markdown
Author

Great @jlarson4 , happy to contribute!

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.

2 participants