Skip to content

[Proposal] Remove or wire the unused eps_attr config field #1376

Description

@RecreationalMath

Proposal

Decide whether TransformerBridgeConfig.eps_attr (transformer_lens/config/transformer_bridge_config.py:86) should be wired into NormalizationBridge or removed. It is set by 8 architecture adapters, asserted as a stored literal in 6 adapter unit tests, and documented as a live knob in 7 doc files, but nothing reads it.

Motivation

The field was introduced in #1096 with a real read site in NormalizationBridge, removed in #1119 when the manual norm computation was replaced by direct delegation to the HF module. Since then it has had zero read sites in any access form (attribute access, getattr-by-string, or dict access).

Epsilon is sourced correctly without it today:

  • cfg.eps is mapped from the HF config in model_bridge/sources/transformers.py:167-173 (rms_norm_eps, then layer_norm_eps, then layer_norm_epsilon).
  • The autograd/folding hook path uses a hardcoded introspection chain in generalized_components/normalization.py:134-140 (module .eps, then .variance_epsilon, then cfg.eps). Spot-checked against transformers 5.8.1: LlamaRMSNorm and CohereLayerNorm resolve via .variance_epsilon, GPT-BigCode's nn.LayerNorm via .eps.

Meanwhile, the field keeps propagating into new adapters, tests, and docs as if it were live (my own GPT-OSS adapter tests in #1341 assert it too). One set value is already wrong by construction: gpt_bigcode sets eps_attr="layer_norm_epsilon", an HF config field name rather than a module attribute (its norms are nn.LayerNorm, which stores .eps), so the old read pattern would silently fall back to its 1e-5 default if the field were re-wired as-is.

Pitch

Option A, remove it: delete the config field, the 8 adapter assignments, the 6 test assertions, and the doc/template mentions. Matches the #1119 design where normalization output comes from the HF module itself.

Alternatives

Option B, wire it: make the normalization.py:134-140 introspection consult cfg.eps_attr first. Worth noting this would only affect the hook_scale/hook_normalized intermediates, since the forward output is pure HF delegation, and the gpt_bigcode value would need fixing first.

Checklist

  • I have checked that there is no similar issue in the repo

Activity

  1. jlarson4 commented on Jun 10, 2026

    @jlarson4
    Collaborator

    This all makes sense, I have verified on my end. I lean towards removal/cleanup, but if you have a case for wiring it, I am open to that option.

  2. added
    low-priorityMaintainers are not prioritising this work currently.
    complexity-simpleSimple issues, which may be good for beginners
    refactorChanging something with the code that will either affect external user, or contributors
    on Jun 10, 2026
  3. RecreationalMath commented on Jun 10, 2026

    @RecreationalMath
    ContributorAuthor

    Agreed, removal it is. I do not see a case for wiring it, the only reader was already removed, and a rewire would only touch the no-grad hook intermediates, nothing user-facing. I will send the removal PR (config field, the 8 adapter assignments, the 6 test assertions, and the doc/template mentions).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

breaking-changecomplexity-simpleSimple issues, which may be good for beginnerslow-priorityMaintainers are not prioritising this work currently.refactorChanging something with the code that will either affect external user, or contributors

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions