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

Allow frozen parameters with requires_grad=False #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Allow frozen parameters with requires_grad=False #5

wants to merge 1 commit into from

Conversation

andrewli77
Copy link

Hi Lucas, thanks for open-sourcing your code for this project. I think it would be nice to be able to manually freeze some network parameters (I was doing this in the context of transfer learning). Currently, the gradient norm is computed over parameters with requires_grad=False as well and p.grad is None.

@lcswillems
Copy link
Owner

Hi @andrewli77,
Thank you for your PR! How would you define the value of requires_grad?

@andrewli77
Copy link
Author

Hi Lucas, sorry for the late reply. requires_grad is an attribute for every parameter of nn.Module which defaults to true. My code sets it by subclassing ACModel and running

for param in self.rnn.parameters():
         param.requires_grad = False

@lcswillems
Copy link
Owner

Hi @andrewli77,

I completely forgot about your PR. I am wondering if your modifications change something.

Your first modification:

grad_norm = sum(p.grad.data.norm(2).item() ** 2 for p in self.acmodel.parameters() if p.requires_grad) ** 0.5

If p.requires_grad is false, then p.grad.data would be an array of zeros, right? In this case, removing if p.requires_grad would not change anything right?

Your second modification:

torch.nn.utils.clip_grad_norm_([p for p in self.acmodel.parameters() if p.requires_grad], self.max_grad_norm)

If p.requires_grad is false, then p.grad.data would be an array of zeros and therefore clipping would change nothing.

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