Skip to content

Conversation

@albanD
Copy link
Collaborator

@albanD albanD commented Nov 21, 2019

Stack from ghstack:

Differential Revision: D18665461

Copy link
Contributor

@vincentqb vincentqb left a comment

Choose a reason for hiding this comment

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

LGTM

@vincentqb vincentqb added the module: bc-breaking Related to a BC-breaking change label Nov 22, 2019
@vincentqb
Copy link
Contributor

vincentqb commented Nov 22, 2019

Flagged as BC breaking.

As we discussed offline, since the gradient used to be modified in place, the gradient might have been different after running the optimizer.step. This is no longer the case.

albanD added a commit to albanD/pytorch that referenced this pull request Nov 25, 2019
@facebook-github-bot facebook-github-bot deleted the gh/albanD/11/head branch December 10, 2019 15:18
wuhuikx pushed a commit to wuhuikx/pytorch that referenced this pull request Jan 30, 2020
…place

Summary: Pull Request resolved: pytorch#30257

Test Plan: Imported from OSS

Differential Revision: D18665461

Pulled By: albanD

fbshipit-source-id: cfdafef919468a41007881b82fd288b7128baf95
pytorchmergebot pushed a commit that referenced this pull request Oct 9, 2024
Fixes #38006

The note was originally added in #30257, which tried to ensure that the gradient wasn't modified in the optimizer. This note creates more confusion than is helpful, so removing it is better than leaving it in, especially because most uses of closure that I know _does_ modify the grads.

Pull Request resolved: #137535
Approved by: https://github.com/albanD
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module: bc-breaking Related to a BC-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants