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

In 1.0.4, training_step is never called when optimizer_step is overriden #4447

Closed
catalys1 opened this issue Oct 30, 2020 · 5 comments
Closed
Assignees
Labels
bug Something isn't working help wanted Open to be worked on priority: 0 High priority task
Milestone

Comments

@catalys1
Copy link
Contributor

catalys1 commented Oct 30, 2020

🐛 Bug

Something changed from 1.0.3 to 1.0.4: in 1.0.3 I was able to override optimizer_step in my LightningModule and everything worked fine. But in 1.0.4, when I override optimizer_step it causes training_step to be skipped.

Please reproduce using the BoringModel and post here

BoringModel replication

Expected behavior

Defining optimizer_step should not cause training_step to be skipped over. This is definitely a problem.

@catalys1 catalys1 added bug Something isn't working help wanted Open to be worked on labels Oct 30, 2020
@jwspaeth
Copy link

Also ran into this bug. Fixed in my case by passing optimizer.step the optimizer_closure, as they do in LightningModule here.

@catalys1
Copy link
Contributor Author

Thanks, that seems to be working. If this is going to be the required way of handling it (unlike in previous releases) then I think it needs to be clearly pointed out in the documentation. Otherwise, hopefully it can be fixed before the next maintenance update.

@ananyahjha93
Copy link
Contributor

yes, I think the docs are a bit old, I am updating them in the fix. Apart from that just making sure that passing optimizer_closure is the only issue here.

@edenlightning
Copy link
Contributor

@SeanNaren please update what is left TODO here

@edenlightning edenlightning added this to the 1.0.x milestone Nov 3, 2020
@SeanNaren
Copy link
Contributor

This should be closed, the docs have been cleared up in #4455, ensuring that users know to pass the optimizer_closure to the optimizer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Open to be worked on priority: 0 High priority task
Projects
None yet
Development

No branches or pull requests

5 participants