Skip to content

Reanchoring tests and benchmarks - #1603

Merged
jlarson4 merged 5 commits into
dev-4.xfrom
refactor/reanchoring-tests-and-benchmarks
Aug 5, 2026
Merged

Reanchoring tests and benchmarks#1603
jlarson4 merged 5 commits into
dev-4.xfrom
refactor/reanchoring-tests-and-benchmarks

Conversation

@jlarson4

@jlarson4 jlarson4 commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Description

Re-anchors TransformerBridge's entire test and benchmark suite off the live HookedTransformer oracle, so the upcoming Hooked* removal can land without deleting the proof that the bridge is correct. After this PR, no test or benchmark constructs a HookedTransformer. The references come from three sources instead:

  1. Frozen goldens – reference numerics (state dicts, logits, activation snapshots, hook manifests, ablation anchors) captured from HookedTransformer across 6 models × 4 processing configs, hosted on the HF Hub (lars4776/TL-Goldens) and pinned by commit revision in tests/goldens.py. The gpt2 golden anchors reproduce the published Main-Demo values (3.999 → 5.453) exactly.
  2. Raw HuggingFace models – unprocessed-numerics and training parity compare against AutoModelForCausalLM (fp32, eager) directly.
  3. Intrinsic contracts – properties that need no reference model (self-patch is a logits no-op, gated hooks capture the residual copy, bwd-hook gradients equal retain_grad ground truth, batch equals per-row forward).

Main changes:

  • Golden infrastructure: capture script (scripts/capture_ht_goldens.py), revision-pinned loader + GoldenCell accessor (tests/goldens.py), skip-gated session fixtures.
  • Benchmarks (verify_models phases): every benchmark takes plain reference data (reference_logits, reference_hooks, reference_activations, …) instead of a reference_model; Phase 3's HF log_softmax fallback promoted to the only equivalence path; Phases 5/6 retired; --no-ht-reference removed; memory estimates recalibrated.
  • Tests: ~40 files migrated onto goldens/HF/intrinsic anchors; 36 HT-only test files deleted; dead HT fixtures removed from conftests. A long-skipped adapter=None ↔ adapter weight-processing equivalence test is resurrected as an executable spec against the frozen state dicts (1e-5, fp64-verified noise floor), and the skipped backward-hook gradient test is resurrected against autograd.
  • Product fixes found by the re-anchoring: get_bridge_params/tl_parameters() returned a wrongly-reshaped W_V for joint-QKV models and omitted LN params (corrupting SVDInterpreter results); ActivationCache.remove_batch_dim/apply_slice_to_batch_dim mishandled broadcast cache entries on batch>1. Weight-conversion and factory signatures widened from HookedTransformerConfig to TransformerLensConfig.
  • lit/ rewritten onto the bridge: HookedTransformerLIT*TransformerLensLIT* (legacy aliases kept for one transition release); model validation is protocol-based; from_pretrained boots a compat-mode bridge.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes (unit 5093, integration/acceptance/benchmarks 1083+ against the goldens; mypy clean)
  • I have not rewritten tests relating to key interfaces which would affect backward compatibility

@sohv

sohv commented Aug 4, 2026

Copy link
Copy Markdown

Drive-by note from reading this against tests/QUARANTINES.md — not a blocker, just something that's easy to miss in a changeset this size.

This PR removes 35 test files, and six of them are inventoried in QUARANTINES.md, which isn't in the changeset. That leaves 10 rows pointing at files that no longer exist (line numbers on dev-4.x):

QUARANTINES.md Deleted file Section
:144, :167 acceptance/test_hooked_transformer.py whole-file quarantine + 2 dtype skipifs
:71, :72 acceptance/test_multi_gpu.py hardware (2+ / 4+ CUDA)
:163 integration/test_tensor_extraction_consistency.py technical debt
:164 integration/test_tokenization_methods.py technical debt
:14, :65, :66 unit/components/test_attention.py bitsandbytes, CUDA half/bf16, MPS+torch 2.8.0 regression env
:15 unit/factories/test_mlp_factory.py bitsandbytes

The file's own header says it's an "Inventory of every skip / skipif / xfail in tests/", and the ⚠️ convention is that whole-module pytestmark skips get a row, so keeping it in sync in the same PR seems in keeping with how it's maintained.

Two of these are worth a second look beyond bookkeeping, since deleting the test also drops the documented coverage note:

  • unit/components/test_attention.py:492 is the MPS + torch 2.8.0 regression env for pytorch#161640.
  • test_attention.py:130 / test_mlp_factory.py:40 are the only bitsandbytes-gated tests in the inventory.

If that coverage moves somewhere under the new goldens layout, updating the rows to point at the new home would keep the inventory useful; if it's intentionally dropped, deleting the rows says so explicitly.

Also dangling after this lands, both referencing tests/acceptance/test_hooked_transformer.py by path: the root AGENTS.md:34 warning block, and docs/source/content/contributing.md (the admonition at the top and the "Quarantined tests" section).

Happy to send a follow-up PR that just syncs QUARANTINES.md and those two docs once this merges, if that's easier than folding it in here- I have the un-quarantining of test_hooked_encoder.py / test_hooked_encoder_decoder.py in flight anyway (they turn out to be green; the "CI test pollution" label was covering several unrelated things). Those two files aren't touched by this PR so there's no conflict either way.

@jlarson4
jlarson4 merged commit 2985420 into dev-4.x Aug 5, 2026
25 checks passed
jlarson4 pushed a commit that referenced this pull request Aug 5, 2026
…inventory (#1606)

Two changes that both make tests/QUARANTINES.md true again.

1. Un-quarantine the encoder acceptance suites (#1605)

test_hooked_encoder.py and test_hooked_encoder_decoder.py were skipped by
3efbd6e ("Cleanup (#1129)", 2025-11-15) as "CI test pollution". There is no
order-dependence in either: T5 is 22 passed / 1 skipped / 0 failed standalone
and inside the full tier, and BERT had a single failure, test_bert_block.

That failure was a stale API assumption, not pollution. BertLayer.forward
returned tuple[torch.Tensor] in transformers 4.56 and returns a bare Tensor in
5.x, so huggingface_block(embed_out)[0] went from unpacking the tuple to
indexing the batch dimension ([1,8,768] vs [8,768]). The module was quarantined
while the pin was still transformers>=4.56 — the test was green at the time —
and broke at the v5 floor bump in 8e8d9d4 (#1315, 2026-05-19), unobserved
because the module was dark. The sibling [0] unpacks at :58/:88/:108 are still
correct; BertSelfAttention and BertEncoder kept their contracts.

Restores 40 tests (36 runnable, 4 environment-gated).

2. Sync the inventory with the files #1603 removed

The reanchoring in #1603 deleted 35 test files without touching
tests/QUARANTINES.md, leaving 10 rows across 6 deleted files:
test_hooked_transformer.py (x2), test_multi_gpu.py (x2),
test_tensor_extraction_consistency.py, test_tokenization_methods.py,
unit/components/test_attention.py (x3) and test_mlp_factory.py. Those rows are
removed. With test_hooked_transformer.py gone, no whole-file quarantines remain,
so the "⚠️ Technical debt — whole-file" section is dropped entirely.

AGENTS.md and contributing.md both asserted the HookedTransformer acceptance
suite was quarantined and named the two encoder files alongside it; both are
corrected. Also fixes three drifted line numbers in the inventory and notes that
the multi-GPU validation record refers to a since-removed file.

Verified: tests/acceptance -m "not slow" -> 230 passed, 31 skipped, 0 failed.
Every path referenced in QUARANTINES.md now exists, and every module-level
skip marker in tests/ is accounted for.

Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
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