Skip to content

Removing grad tensors creation if tensor does not require grad.#3190

Closed
tudorcebere wants to merge 8 commits intoOpenMined:masterfrom
tudorcebere:master
Closed

Removing grad tensors creation if tensor does not require grad.#3190
tudorcebere wants to merge 8 commits intoOpenMined:masterfrom
tudorcebere:master

Conversation

@tudorcebere
Copy link
Contributor

@tudorcebere tudorcebere commented Mar 13, 2020

I observed that syft is creating grad tensors for all tensors, even if freezed and calling backwards on them. The grad tensors are useful with unfreezed layers only, right? The lead was from the autograd[0] documentation. If I remove the grad_tensor creation for the freezed layers and leave it None, it seems to work fine. Any advices?

This PR fixes #3180.
[0]: https://pytorch.org/docs/stable/autograd.html

Copy link
Member

@TTitcombe TTitcombe left a comment

Choose a reason for hiding this comment

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

Looks good. A couple of points:

  1. Can you add a test to confirm that we can now send a model with frozen layers
  2. If you add the line Fixes #3180 to the description, the issue will automatically be closed when the PR gets merged

@tudorcebere
Copy link
Contributor Author

Should be fine now.

Copy link
Contributor

@LaRiffle LaRiffle left a comment

Choose a reason for hiding this comment

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

Great job!

@tudorcebere tudorcebere requested a review from a team March 15, 2020 17:02
@karlhigley
Copy link
Contributor

@tudorcebere Looks good, but seems like this PR picked up some Plan changes somehow. Are those supposed to be in this PR?

@karlhigley karlhigley added the Type: Bug 🐛 Some functionality not working in the codebase as intended label Mar 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: Bug 🐛 Some functionality not working in the codebase as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Runtime Error asking all parameters to have requires_grad=True

4 participants