-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
optimizer clean up #4658
optimizer clean up #4658
Conversation
Hello @tchaton! Thanks for updating this PR.
Comment last updated at 2020-11-30 23:35:35 UTC |
…chLightning/pytorch-lightning into bugfix/4572_optimizer_step
…chLightning/pytorch-lightning into bugfix/4572_optimizer_step
Co-authored-by: Carlos Mocholí <carlossmocholi@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @tchaton. I know there is a lot of discussion if this breaks pytorch-esque lightning but I think this is was needed. The amount of bugs/questions that boil down to incorrect override/usage of optimizer_step that this PR solves are huge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still reading...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is a huge change so let's hope we did not miss something...
What does this PR do?
This PR is a first attempt at cleaning out optimization by introducing LightningOptimizer.
Here is how SGD will get printed.
Fixes #4663
Before submitting
PR review
Anyone in the community is free to review the PR once the tests have passed.
Before you start reviewing make sure you have read Review guidelines. In in short, see following bullet-list:
Did you have fun?
Make sure you had fun coding 🙃