TransformerLensOrg / TransformerLens

A library for mechanistic interpretability of GPT-style language models
https://transformerlensorg.github.io/TransformerLens/
MIT License
1.17k stars 241 forks source link

[Bug Report] Missing `.items()` in `HookedRootModule.hooks` #611

Closed dtch1997 closed 1 month ago

dtch1997 commented 1 month ago

Describe the bug Incorrect use of self.hook_dict in HookedRootModule.hooks when iterating over backward hooks.

Code example

def noop(act, hook):
    pass

with model.hooks(bwd_hooks = [(lambda x: True, noop)]): 
    pass

Additional context https://github.com/TransformerLensOrg/TransformerLens/blame/5a374ec4b33cec6281b37494175d14f06c75dcfd/transformer_lens/hook_points.py#L306-L313

Checklist

bryce13950 commented 1 month ago

Thank you for opening this. We need to spend some time investigating this before we fix it. This file specifically has recently had quite a few changes in the pending 2.0 release. There is a possibility that the bug is fixed there, so a little bit of work needs to be done to see the full scope.

neelnanda-io commented 1 month ago

This actually is fixed in v2.0 https://github.com/TransformerLensOrg/TransformerLens/blame/295aca59ef73971e97bc7df5723f7f701de8497b/transformer_lens/hook_points.py#L374

On Mon, 27 May 2024 at 19:46, Bryce Meyer @.***> wrote:

Thank you for opening this. We need to spend some time investigating this before we fix it. This file specifically has recently had quite a few changes in the pending 2.0 release. There is a possibility that the bug is fixed there, so a little bit of work needs to be done to see the full scope.

— Reply to this email directly, view it on GitHub https://github.com/TransformerLensOrg/TransformerLens/issues/611#issuecomment-2133931266, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASRPNKLYE7D5JBY4U4WUDA3ZEN5QHAVCNFSM6AAAAABILBYWTSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMZTHEZTCMRWGY . You are receiving this because you are subscribed to this thread.Message ID: @.***>

bryce13950 commented 1 month ago

I'll get it back ported into 1.x later today.

bryce13950 commented 1 month ago

This has been resolved in 1.19, and a nice little test has been added to this to make sure this block is now covered for the future.