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

Add on_after_backwards callback option #3657

Closed
rbracco opened this issue Sep 25, 2020 · 7 comments · Fixed by #4379
Closed

Add on_after_backwards callback option #3657

rbracco opened this issue Sep 25, 2020 · 7 comments · Fixed by #4379
Labels
feature Is an improvement or enhancement help wanted Open to be worked on

Comments

@rbracco
Copy link
Contributor

rbracco commented Sep 25, 2020

🚀 Feature

Add an on_after_backward callback option.

Motivation

Currently there are no callbacks for after the backwards step so non-essential code like logging gradients clutters the LightningModule.

Pitch

Expand the callback options to include a hook for on_after_backward() to execute callbacks immediately after the backwards pass

Alternatives

Sticking my non-essential gradient logging code in my LightningModule

@rbracco rbracco added feature Is an improvement or enhancement help wanted Open to be worked on labels Sep 25, 2020
@awaelchli
Copy link
Contributor

useful!

@FelixLorenz
Copy link

FelixLorenz commented Sep 28, 2020

We nearly need the same feature: on_before_backward as an abstract method in the Callback base class would be awesome!

Currently we are using the same alternative (in the training_step) but this couples one last bit of our custom callback with the code of the lightning module...

@rohitgr7
Copy link
Contributor

rohitgr7 commented Oct 1, 2020

maybe we should add all the optimizer-related hooks to callbacks too if it makes sense.
What do you think @awaelchli?

@awaelchli
Copy link
Contributor

awaelchli commented Oct 2, 2020

@rohitgr7 which hooks do you have in mind?
@FelixLorenz it shouldn't be abstract

@rohitgr7
Copy link
Contributor

rohitgr7 commented Oct 2, 2020

@awaelchli all of them? on_after_backward, on_before_backward, on_before_zero_grad.

@awaelchli
Copy link
Contributor

I think that would be nice to have.

@rohitgr7
Copy link
Contributor

rohitgr7 commented Oct 2, 2020

okay, will add them.
Waiting for more approvals to see if these hooks are good to add or not.
@PyTorchLightning/core-contributors

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Is an improvement or enhancement help wanted Open to be worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants