passing batch outputs to on_train_batch_end #4369

swethmandava · 2020-10-26T13:18:21Z

What does this PR do?

Passes batch outputs to on_train_batch_end instead of epoch end outputs

Before submitting

Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
Did you read the contributor guideline, Pull Request section?
Did you make sure your PR does only one thing, instead of bundling different changes together? Otherwise, we ask you to create a separate PR for every change.
Did you make sure to update the documentation with your changes?
Did you write any new necessary tests?
Did you verify new and existing tests pass locally with your changes?
If you made a notable change (that affects users), did you update the CHANGELOG?

PR review

[Y] Is this pull request ready for review? (if not, please submit in draft mode)

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

pep8speaks · 2020-10-26T13:18:24Z

Hello @swethmandava! Thanks for updating this PR.

In the file pytorch_lightning/trainer/training_loop.py:

Line 256:85: W504 line break after binary operator

In the file tests/models/test_hooks.py:

Line 140:121: E501 line too long (121 > 120 characters)

Comment last updated at 2021-01-26 12:16:43 UTC

edenlightning · 2020-10-27T13:35:14Z

Thanks for the PR!

Please take a second look as it seems test are failing...

Borda · 2020-11-03T13:58:46Z

@swethmandava could you please check failing tests and also add a test for this change?

Update

swethmandava · 2020-11-11T20:55:15Z

@Borda @edenlightning fixed pytest errors. it now fails with /bin/bash: /root/miniconda3/envs/lightning/lib/libtinfo.so.6: no version information available (required by /bin/bash) seems to be conda version related - can you help?

BotScutters · 2020-12-30T18:33:46Z

Hey all, I'm working on a callback that would generate a plot of model outputs each epoch and encountering an issue that appears to be related to this. I'd expect I should be able to grab the model outputs from the outputs arg of on_training_batch_end, on_validation_batch_end, etc., but this shows comes in as None. A bit of googling brought me here, and it seems this issue has gone a little stale.

Is this issue still being worked on? If not, is there a suggested workaround? Thanks!

swethmandava · 2021-01-14T01:25:17Z

@SeanNaren @tchaton can this be merged?

tchaton

Would it be possible to have a test ?

SeanNaren · 2021-01-20T18:11:27Z

Apologies for the stale! A simple boring model test that ensures that if the user overrides this hook, they retrieve the appropriate model outputs would be nice :)

Similar to this: https://github.com/PyTorchLightning/pytorch-lightning/blob/master/tests/models/test_hooks.py#L52

thanks for working on this!

awaelchli · 2021-01-20T18:02:03Z

pytorch_lightning/trainer/training_loop.py

                batch_output.training_step_output_for_epoch_end,
                self.early_stopping_accumulator,
                self.checkpoint_accumulator,
            )
-
            # hook
            # TODO: add outputs to batches


anyone knows what this todo means?

swethmandava · 2021-01-22T02:58:48Z

Apologies for the stale! A simple boring model test that ensures that if the user overrides this hook, they retrieve the appropriate model outputs would be nice :)

Similar to this: https://github.com/PyTorchLightning/pytorch-lightning/blob/master/tests/models/test_hooks.py#L52

thanks for working on this!

Done!

tchaton

LGTM !

SeanNaren · 2021-01-26T13:02:24Z

Thanks @swethmandava for doing this and apologies on the delay, fell through the cracks a few times...

Borda · 2021-01-26T23:48:00Z

tests/models/test_hooks.py

+
+    class LoggingCallback(pl.Callback):
+
+        def on_train_epoch_end(self, trainer, pl_module):


@SeanNaren flake8 found this bug, but it was still merged, could pls fix it in #5666
most likely on_train_epoch_end >> on_train_epoch_start as *_end is here twice

* passing batch outputs to on_train_batch_end * styling * updating epoch end logic * also condition on on_train_epoch_end hooks * more readable * pep8 * pep8 * readability suggestion accepted Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> * adding test_training_epoch_end_metrics_collection_on_override test * fix formatting * fix formatting Co-authored-by: Swetha Mandava <smandava@nvidia.com> Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> Co-authored-by: Sean Naren <sean.narenthiran@gmail.com> Co-authored-by: Roger Shieh <sh.rog@protonmail.ch> (cherry picked from commit 5fcca4e)

passing batch outputs to on_train_batch_end

0bc92df

swethmandava requested review from ananyahjha93, awaelchli, Borda, justusschock, nateraw, SeanNaren, tchaton, teddykoker and williamFalcon as code owners October 26, 2020 13:18

mergify bot requested a review from a team October 26, 2020 13:19

styling

c738b14

edenlightning added this to the 1.0.x milestone Oct 27, 2020

rohitgr7 self-requested a review October 27, 2020 14:31

Borda added the waiting on author Waiting on user action, correction, or update label Nov 3, 2020

Merge pull request #1 from PyTorchLightning/master

282ce57

Update

edenlightning modified the milestones: 1.0.x, 1.0.7 Nov 10, 2020

updating epoch end logic

725b5e3

Borda modified the milestones: 1.0.7, 1.0.x Nov 11, 2020

edenlightning modified the milestones: 1.0.x, 1.0.7 Nov 13, 2020

Borda modified the milestones: 1.0.7, 1.0.x Nov 13, 2020

rohitgr7 mentioned this pull request Nov 15, 2020

What are the outputs in the on_train_batch_end callback? #4689

Closed

rsaite mentioned this pull request Jan 14, 2021

Pass epoch outputs to callback hooks on_validation_epoch_end and on_test_epoch_end #5508

Closed

s-rog linked an issue Jan 15, 2021 that may be closed by this pull request

Empty "outputs" argument in the on_train_batch_end() method of Callback #5517

Closed

s-rog approved these changes Jan 15, 2021

View reviewed changes

Merge branch 'master' into master

5907b59

hackgoofer mentioned this pull request Jan 18, 2021

[feat] pytorch lightning integration - training facebookresearch/mmf#734

Closed

8 tasks

tchaton reviewed Jan 20, 2021

View reviewed changes

awaelchli reviewed Jan 21, 2021

View reviewed changes

Swetha Mandava added 2 commits January 21, 2021 18:56

adding test_training_epoch_end_metrics_collection_on_override test

fb24a4d

merge conflict

3d21ad6

Swetha Mandava added 2 commits January 21, 2021 19:05

fix formatting

fa586ae

fix formatting

00846db

hackgoofer mentioned this pull request Jan 23, 2021

[feat] PL mvp0: training facebookresearch/mmf#748

Closed

17 tasks

SeanNaren approved these changes Jan 26, 2021

View reviewed changes

Merge branch 'master' into master

530119f

tchaton approved these changes Jan 26, 2021

View reviewed changes

SeanNaren added the ready PRs ready to be merged label Jan 26, 2021

Borda requested a review from awaelchli January 26, 2021 12:49

SeanNaren merged commit 5fcca4e into Lightning-AI:master Jan 26, 2021

Borda reviewed Jan 26, 2021

View reviewed changes

Borda mentioned this pull request Jan 26, 2021

new section + legacy 1.1.6 #5666

Merged

12 tasks

awaelchli mentioned this pull request May 4, 2021

[1/2] Deprecate outputs in on_train_epoch_end hooks #7339

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

passing batch outputs to on_train_batch_end #4369

passing batch outputs to on_train_batch_end #4369

swethmandava commented Oct 26, 2020 •

edited

Loading

pep8speaks commented Oct 26, 2020 •

edited

Loading

edenlightning commented Oct 27, 2020

Borda commented Nov 3, 2020

swethmandava commented Nov 11, 2020

BotScutters commented Dec 30, 2020

swethmandava commented Jan 14, 2021 •

edited

Loading

tchaton left a comment

SeanNaren commented Jan 20, 2021

awaelchli Jan 20, 2021

swethmandava commented Jan 22, 2021

tchaton left a comment

SeanNaren commented Jan 26, 2021

Borda Jan 26, 2021 •

edited

Loading


		class LoggingCallback(pl.Callback):

		def on_train_epoch_end(self, trainer, pl_module):

passing batch outputs to on_train_batch_end #4369

passing batch outputs to on_train_batch_end #4369

Conversation

swethmandava commented Oct 26, 2020 • edited Loading

What does this PR do?

Before submitting

PR review

Did you have fun?

pep8speaks commented Oct 26, 2020 • edited Loading

Comment last updated at 2021-01-26 12:16:43 UTC

edenlightning commented Oct 27, 2020

Borda commented Nov 3, 2020

swethmandava commented Nov 11, 2020

BotScutters commented Dec 30, 2020

swethmandava commented Jan 14, 2021 • edited Loading

tchaton left a comment

Choose a reason for hiding this comment

SeanNaren commented Jan 20, 2021

awaelchli Jan 20, 2021

Choose a reason for hiding this comment

swethmandava commented Jan 22, 2021

tchaton left a comment

Choose a reason for hiding this comment

SeanNaren commented Jan 26, 2021

Borda Jan 26, 2021 • edited Loading

Choose a reason for hiding this comment

swethmandava commented Oct 26, 2020 •

edited

Loading

pep8speaks commented Oct 26, 2020 •

edited

Loading

swethmandava commented Jan 14, 2021 •

edited

Loading

Borda Jan 26, 2021 •

edited

Loading