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.

Sign up for GitHub

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

Jump to bottom

Add learned activations L0 norm metric #91

Merged
merged 9 commits into from
Nov 27, 2023
Merged

Add learned activations L0 norm metric #91

merged 9 commits into from
Nov 27, 2023

Conversation

lucyfarnik
Copy link
Contributor

Closes issue #87

@lucyfarnik
Copy link
Contributor Author

Something to note on this PR: the L0NormMetric.create_progress_bar_postfix method isn't implemented yet. The abstract class didn't provide any info on what this method should do so I didn't have anything to go off of here.

alan-cooney
alan-cooney requested changes Nov 22, 2023
View reviewed changes
Copy link
Collaborator

@alan-cooney alan-cooney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The L0 loss function already logs to wandb, so this doesn't add anything I'm afraid.

However if we decide we want L0 on the reconstructed neurons it would be reasonable to add this.

Btw I've also removed create_progress_bar_postfix and renamed create_weights_and_biases_log to just calculate.

@lucyfarnik
Copy link
Contributor Author

I'm slightly confused, I'm pretty sure the only Lp norm that's getting logged to wandb is the L1? At least this was what I was seeing when running the code before I added these features in:
Screenshot 2023-11-23 at 7 44 32 AM

Re removing create_progress_bar_postfix: sounds great! I think that was the right call

alan-cooney
alan-cooney requested changes Nov 23, 2023
View reviewed changes
Copy link
Collaborator

@alan-cooney alan-cooney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore my previous review - was mixing things up!

I've added some suggestions.

I've also merged in main already and fixed the merge conflicts for you, as that was a result of me refactoring.

sparse_autoencoder/metrics/train/l0_norm_metric.py Outdated Show resolved Hide resolved
sparse_autoencoder/metrics/train/tests/test_abstract_metric.py Outdated Show resolved Hide resolved
sparse_autoencoder/metrics/train/tests/test_l0_norm_metric.py Outdated Show resolved Hide resolved
@alan-cooney alan-cooney changed the title Added L0 norm metric Add learned activations L0 norm metric Nov 27, 2023
@alan-cooney alan-cooney merged commit 0b3337b into ai-safety-foundation:main Nov 27, 2023
HoagyC pushed a commit to HoagyC/sparse_autoencoder that referenced this pull request Dec 13, 2023
@lucyfarnik @alan-cooney
Add learned activations L0 norm metric (ai-safety-foundation#91)
c38860d 

---------

Co-authored-by: Alan Cooney <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alan-cooney alan-cooney alan-cooney approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lucyfarnik @alan-cooney