Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Proposal] Type hint support for self.model in ActivationCache #830

Open
1 task done
Ja1Zhou opened this issue Jan 14, 2025 · 1 comment
Open
1 task done

[Proposal] Type hint support for self.model in ActivationCache #830

Ja1Zhou opened this issue Jan 14, 2025 · 1 comment
Labels
complexity-moderate Moderately complicated issues for people who have intermediate experience with the code
Milestone

Comments

@Ja1Zhou
Copy link

Ja1Zhou commented Jan 14, 2025

Proposal

I propose to do a simple refractoring of the class HookedTransformer to support type hints for the model attribute in the ActivationCache class.

Motivation

I am frastrated by the lack of type hinting for self.model in the ActivationCache class. I think the current state is that we want to avoid circular imports, but I think we can do better.

Pitch

Create a class, say HookedTransformerMixin, that holds all methods without accessing to the definition of ActivationCache. Currently only the function run_with_cache access the ActivationCache class.

In transformer_lens/ActivationCache.py, we could import HookedTransformerMixin to provide type hint. In transformer_lens/HookedTransformer.py, HookedTransformer could inherit HookedTransformerMixin and import from ActivationCache.

Checklist

  • I have checked that there is no similar issue in the repo (required)
@bryce13950
Copy link
Collaborator

There is a low priority task to separate the HookedRootModule into it's own file, which could then be used as described, as well as making it easier for people to boot TransformerLens while receiving an instance of HookedRootModule based on config without necessarily needing to know which HookedModule needs to be manually instantiated. That is currently on the docket for 4.0, but we can probably work it into 3.0 in order to address this as well, providing that it doesn't turn into a large project.

@bryce13950 bryce13950 added complexity-high Very complicated changes for people to address who are quite familiar with the code complexity-moderate Moderately complicated issues for people who have intermediate experience with the code and removed complexity-high Very complicated changes for people to address who are quite familiar with the code labels Jan 18, 2025
@bryce13950 bryce13950 added this to the 3.0 milestone Jan 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity-moderate Moderately complicated issues for people who have intermediate experience with the code
Projects
None yet
Development

No branches or pull requests

2 participants