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

Add direction and direction_neuron objectives #6

Merged
merged 8 commits into from
Jun 23, 2020

Conversation

ndey96
Copy link
Contributor

@ndey96 ndey96 commented Jun 15, 2020

Hi @greentfrapp,

Thanks so much for making this repo! It has been a great help for me. For my use-case I needed the direction and direction_neuron objectives so I added them into lucent. I also included two demo files but let me know if they should be rolled into a docstring instead. This PR also lays the groundwork to reproduce the activation atlas notebooks from lucid. Would love to hear your thoughts :)

@greentfrapp
Copy link
Owner

Thank you @ndey96 for the contribution and I'm really glad that you found the library helpful!

The new direction objectives look good! Just a few quick things

  • Instead of the demo scripts, add a few key lines in the docstring for the new functions e.g.
# with resnet18 (possible to change this for inceptionv1? since that's what most of our demos are based on by default)
direction = torch.rand(256) * 1000 
obj = objectives.direction(layer='layer3', direction=direction)
  • Remove the docstring chunk for direction_neuron that starts with "Defaults to the center neuron." and point the reader to the same docstring chunk in the neuron objective
  • Add your new tests at the end of test_objectives.py instead, since some of the tests are kinda sensitive to the random seed

We should be good to merge after that!

@ndey96
Copy link
Contributor Author

ndey96 commented Jun 22, 2020

@greentfrapp Addressed your comments :) I noticed that a few other PRs were recently merged. Is it possible to bump the version on PyPI once this gets merged?

Thanks!

@greentfrapp
Copy link
Owner

Awesome thank you @ndey96 I'll be merging this!

Yes, bumping the pip version is definitely on my personal ToDo list! Well the ToDo list is worryingly long, but I'm feeling a tiny bit more productive this week so I have hope that it will be bumped by the end of the week. Fingers crossed!

@greentfrapp greentfrapp merged commit 62e9019 into greentfrapp:master Jun 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants