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

Don't reuse nn.ReLU modules in CLIP ResNet #33

Closed
wants to merge 1 commit into from

Conversation

ProGamerGov
Copy link
Contributor

Summary:

Reusing the same ReLU module for multiple layers can make it more difficult for researchers as for example PyTorch's hook system won't work properly on the reused layer modules. I ran into this issue while building and testing interpretability tools on the CLIP models.

This PR doesn't change how any of the models work. It merely makes it possible to access and research each ReLU layer separately. Let me know if I need to make any changes before it can be merged!

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 3, 2022
@ebsmothers
Copy link
Contributor

Thanks for the PR! This looks good to me 😃. Can you also share more about your use case with CLIP? We are still building out the library and any info you can provide would be a helpful datapoint for us.

@facebook-github-bot
Copy link
Contributor

@ebsmothers has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@ProGamerGov
Copy link
Contributor Author

ProGamerGov commented May 3, 2022

@ebsmothers Sure thing! These changes will help make CLIP models more compatible with pytorch/captum's upcoming optimization based interpretability module. Specially the loss objective & activation collection system in the module uses forward hooks, and duplicated modules don't work well with the PyTorch hook system.

The module should hopefully be launching sometime soon!

Suggestion-wise: In the upcoming Captum module I've included the ResNet 50x4 model in the module as two separate parts, and then provide a wrapper that merges them into the full model. It's easier for researchers to work with the text and image portions separately, and can help lower resource use if an individual only needs one part of the model (ex: neuron rendering only requires the image portion of the model). It might be useful to have your CLIP models easily separable into their text and image modules as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants