Skip to content

[BC-breaking] Remove legacy autograd function #22762

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.

Sign up for GitHub

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

Jump to bottom
Closed
yf225 wants to merge 1 commit into from
Closed

[BC-breaking] Remove legacy autograd function #22762

yf225 wants to merge 1 commit into from

Conversation

@yf225
Copy link
Contributor

@yf225 yf225 commented Jul 11, 2019

Legacy autograd function has been deprecated for 3 major releases, and therefore it should okay to remove it completely. This fixes issues such as #16947.

Note that this PR is BC-breaking because legacy autograd function (with non-static forward method) will not be supported after this PR.

@yf225 yf225 added the module: bc-breaking Related to a BC-breaking change label Jul 11, 2019
@yf225 yf225 requested review from ezyang and zou3519 July 11, 2019 20:44
@pytorchbot pytorchbot added module: autograd Related to torch.autograd, and the autograd engine in general module: nn Related to torch.nn module: pybind Related to our Python bindings / interactions with other Python libraries labels Jul 11, 2019
@yf225 yf225 requested a review from gchanan July 11, 2019 20:46
@zou3519
Copy link
Contributor

zou3519 commented Jul 11, 2019

I don't think we ever added a deprecation warning in-code (pytest test/test_autograd.py -v -k "test_legacy_function" doesn't warn) so I'm not sure we can remove this for this upcoming release. Maybe let's add in the deprecation warnings and then remove in the next release?

@soumith
Copy link
Contributor

soumith commented Jul 12, 2019

yea it's deprecated but it was never actually generating warnings due to the amount of legacy code that had already been in the wild.

@yf225 is this cleanup essential / blocker for any refactors you are using? Or did you just routinely cleaned it up?

@ezyang ezyang mentioned this pull request Jul 15, 2019
Closed
@yf225 yf225 mentioned this pull request Jul 16, 2019
Closed
@yf225
Copy link
Contributor Author

yf225 commented Jul 16, 2019

@soumith this cleanup is nice to have but not essential for the refactoring in #22172. I put up another PR for adding the deprecating warning in 1.2: #22922, and I think after a couple more releases we can remove the legacy autograd functions.

@yf225 yf225 closed this Jul 16, 2019
facebook-github-bot pushed a commit that referenced this pull request Jul 18, 2019
@facebook-github-bot
Add warning for legacy autograd function (#22922)
c1c4014 
Summary:
When working on #22762, we discovered that we haven't actually deprecated legacy autograd function. This PR puts up the deprecation warning for 1.2, with the goal to remove legacy function support completely in the near future.
Pull Request resolved: #22922

Differential Revision: D16363916

Pulled By: yf225

fbshipit-source-id: 4b554010a3d1f87a3fa45cc1aa29d019c8f1033c
@Balandat Balandat mentioned this pull request Jul 19, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ezyang ezyang Awaiting requested review from ezyang

@zou3519 zou3519 Awaiting requested review from zou3519

@gchanan gchanan Awaiting requested review from gchanan

Assignees

No one assigned

Labels

module: autograd Related to torch.autograd, and the autograd engine in general module: bc-breaking Related to a BC-breaking change module: nn Related to torch.nn module: pybind Related to our Python bindings / interactions with other Python libraries

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@yf225 @zou3519 @soumith @pytorchbot