jbloomAus / SAELens

Training Sparse Autoencoders on Language Models
https://jbloomaus.github.io/SAELens/
MIT License
453 stars 120 forks source link

Don't allow setting both `d_sae` and `expansion_factor` in config #47

Open chanind opened 7 months ago

chanind commented 7 months ago

These are both ways of setting the size of the SAE

anthonyduong9 commented 1 month ago

I can work on this this week.

anthonyduong9 commented 1 month ago

@chanind I was thinking that as part of this, we should also change

https://github.com/jbloomAus/SAELens/blob/a70822074c94c3bafe8502b54bc6e553483d08e3/sae_lens/config.py#L134

to

expansion_factor: Optional[int] = None

since it's less explicit to check if expansion_factor is 4 than None to check if it's been set. However, people would always have to pass a value for either d_sae or expansion_factor when they instantiate LanguageModelSAERunnerConfig. What do you think?

jbloomAus commented 1 month ago

seems good

chanind commented 1 month ago

I think that sounds reasonable! But just make sure that if both expansion_factor is None, and d_sae is None, then expansion_factor should default to 4 for backwards compatibility. This can be noted in the docstring and also as a comment after expansion_factor: Optional[int] = None # defaults to 4 if d_sae and expansion_factor is None