ref: deprecated results obj, added support for simpler comms (1/n) #3681

williamFalcon · 2020-09-27T18:18:23Z

Here we move to deprecate results objects in favor of simpler syntax. Results obj support will remain until 0.10.0 but not 1.0.0.
(ie: 0.10.0 is 1.0.0 but backwards compatible for anyone who really needs it).

The new pattern is to decouple logging from hooks. That means that step, step_end, epoch_end are independent from logging.

example

def training_step(...):
    loss = ...
    return loss

# equivalent
def training_step(...):
    return {'loss': loss}

# to log
def training_step(...):
    self.log('anything', x, on_step=True, on_epoch=True)

Passing around step results.

In the case the user still needs/wants to do something with the output of each batch, the other hooks are still there.

def training_step(...):
    return {'loss': loss, 'random_thing': [1, 'a', Tensor(), ...]}

def training_epoch_end(self, training_step_outputs):
    for d in training_step_outputs:
        random_thing = d['random_thing']

Hooks and .log are decoupled...

user can log from anywhere

def training_epoch_end(...):
    some_new_val = ...
    self.log('my_new_val', some_new_val)

cc @ananthsub @awaelchli @justusschock

pep8speaks · 2020-09-27T18:18:29Z

Hello @williamFalcon! Thanks for updating this PR.

In the file pytorch_lightning/trainer/logging.py:

Line 72:1: W293 blank line contains whitespace

Comment last updated at 2020-09-28 02:48:30 UTC

pytorch_lightning/core/step_result.py

pytorch_lightning/core/lightning.py

pytorch_lightning/trainer/training_loop.py

mergify · 2020-09-27T22:46:45Z

This pull request is now in conflict... :(

carmocca · 2020-09-27T23:45:00Z

pytorch_lightning/trainer/logging.py

+                    m = f"""
+                    The {{{k}:dict keyword}} was deprecated in 0.9.1 and will be removed in 1.0.0
+                    Please use self.log(...) inside the lightningModule instead.
+
+                    # log on a step or aggregate epoch metric to the logger and/or progress bar
+                    # (inside LightningModule)
+                    self.log('train_loss', loss, on_step=True, on_epoch=True, prog_bar=True)
+                    """


Please, do not use triple quote strings for warnings and logs since they include all the extra whitespace to the left:

>>> import warnings >>> m1 = ( ... "this is " ... "a " ... "test" ... ) >>> warnings.warn(m1) <stdin>:1: UserWarning: this is a test >>> m2 = """ ... this ... is a ... test ... """ >>> warnings.warn(m2) <stdin>:1: UserWarning: this is a test

i disagree with this...

these long strings are more readable to the user when the get this kind of warning.

The problem is not that it is multiline, the problem is is that the whitespace at the beginning depends on the indentation level.

For that case in particular:

<stdin>:1: UserWarning: The {{{k}:dict keyword}} was deprecated in 0.9.1 and will be removed in 1.0.0 Please use self.log(...) inside the lightningModule instead. # log on a step or aggregate epoch metric to the logger and/or progress bar # (inside LightningModule) self.log('train_loss', loss, on_step=True, on_epoch=True, prog_bar=True)

You would have to either remove the indentation:

m = f""" The {{{k}:dict keyword}} was deprecated in 0.9.1 and will be removed in 1.0.0 Please use self.log(...) inside the lightningModule instead. # log on a step or aggregate epoch metric to the logger and/or progress bar # (inside LightningModule) self.log('train_loss', loss, on_step=True, on_epoch=True, prog_bar=True) """

(pretty ugly if you ask me)

or split it and print it with more than one warnings/logs

I agree with @carmocca, if you need an extra line, just use \n

mergify · 2020-09-28T00:20:47Z

This pull request is now in conflict... :(

…es logging from loops

nazim1021 · 2020-10-30T23:58:06Z

cc @ananthsub @awaelchli @justusschock
Hi, after updating to version 1.0.4, i think below approach seems to be not working as desired

def training_step(...):
    return {'loss': loss, 'random_thing': [1, 'a', Tensor(), ...]}

def training_epoch_end(self, training_step_outputs):
    for d in training_step_outputs:
        random_thing = d['random_thing']

training_step_outputs is always empty. On further debugging i find that training_step is never getting called, instead this function training_step_and_backward is called within run_training_batch

Any help appreciated and also apologies if i am making any mistake

williamFalcon · 2020-10-31T00:12:31Z

ok, will fix asap. thanks for reporting!

mind replicating it on a boring model?

@edenafek (p0)

edenlightning · 2020-10-31T00:26:49Z

@nazim1021! Thank you for reporting! I created an issue to track this, would be great if you can try to repro using boring model and paste a colab link in the new issue](#4452)

nazim1021 · 2020-10-31T00:54:55Z

@williamFalcon @edenlightning sure, of course. Thanks for prompt reply
i ll add the colab link to new issue

mergify bot requested a review from a team September 27, 2020 18:19

williamFalcon commented Sep 27, 2020

View reviewed changes

pytorch_lightning/core/step_result.py Outdated Show resolved Hide resolved

Borda reviewed Sep 27, 2020

View reviewed changes

pytorch_lightning/core/lightning.py Outdated Show resolved Hide resolved

mergify bot requested a review from a team September 27, 2020 18:34

Borda reviewed Sep 27, 2020

View reviewed changes

pytorch_lightning/core/lightning.py Outdated Show resolved Hide resolved

mergify bot requested a review from a team September 27, 2020 18:35

williamFalcon changed the title ~~ref: deprecated results obj, added support for simpler comms (1/n)~~ [WIP] ref: deprecated results obj, added support for simpler comms (1/n) Sep 27, 2020

williamFalcon commented Sep 27, 2020

View reviewed changes

pytorch_lightning/trainer/training_loop.py Outdated Show resolved Hide resolved

williamFalcon commented Sep 27, 2020

View reviewed changes

pytorch_lightning/trainer/training_loop.py Outdated Show resolved Hide resolved

williamFalcon changed the title ~~[WIP] ref: deprecated results obj, added support for simpler comms (1/n)~~ ref: deprecated results obj, added support for simpler comms (1/n) Sep 27, 2020

carmocca reviewed Sep 27, 2020

View reviewed changes

williamFalcon force-pushed the lg/wr branch from 53a461d to c1295bf Compare September 28, 2020 00:30

williamFalcon changed the title ~~ref: deprecated results obj, added support for simpler comms (1/n)~~ [WIP] ref: deprecated results obj, added support for simpler comms (1/n) Sep 28, 2020

williamFalcon marked this pull request as draft September 28, 2020 00:46

williamFalcon force-pushed the lg/wr branch from c1295bf to 7e05977 Compare September 28, 2020 00:47

williamFalcon added 12 commits September 27, 2020 22:10

ref: deprecated results obj, added support for simpler comms. Decoupl…

23cdc86

…es logging from loops

ref: deprecated results obj, added support for simpler comms. Decoupl…

c5ab5c2

…es logging from loops

ref: deprecated results obj, added support for simpler comms. Decoupl…

85e3565

…es logging from loops

ref: deprecated results obj, added support for simpler comms. Decoupl…

4ddaf3d

…es logging from loops

ref: deprecated results obj, added support for simpler comms. Decoupl…

c314bf9

…es logging from loops

ref: deprecated results obj, added support for simpler comms. Decoupl…

da641cd

…es logging from loops

fix global step err

5a9b165

fix global step err

57a9a25

fix global step err

8d790f6

fix global step err

47f2404

fix global step err

d4494f0

fix typing err

4f6ad40

williamFalcon force-pushed the lg/wr branch from 2558310 to 4f6ad40 Compare September 28, 2020 02:11

williamFalcon added 2 commits September 27, 2020 22:15

fix str

057c15e

fix typing err

01643ab

williamFalcon marked this pull request as ready for review September 28, 2020 03:19

williamFalcon merged commit ddd1107 into master Sep 28, 2020

williamFalcon changed the title ~~[WIP] ref: deprecated results obj, added support for simpler comms (1/n)~~ ref: deprecated results obj, added support for simpler comms (1/n) Sep 28, 2020

Borda deleted the lg/wr branch September 28, 2020 06:50

Borda added the refactor label Sep 28, 2020

Borda added this to the 0.9.x milestone Sep 28, 2020

edenlightning mentioned this pull request Oct 31, 2020

Training_step outputs not propagated #4452

Closed

edenlightning modified the milestones: 0.10.0, 1.0.x Nov 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ref: deprecated results obj, added support for simpler comms (1/n) #3681

ref: deprecated results obj, added support for simpler comms (1/n) #3681

williamFalcon commented Sep 27, 2020 •

edited

Loading

pep8speaks commented Sep 27, 2020 •

edited

Loading

mergify bot commented Sep 27, 2020

carmocca Sep 27, 2020 •

edited

Loading

williamFalcon Sep 28, 2020

carmocca Sep 28, 2020 •

edited

Loading

Borda Sep 28, 2020

mergify bot commented Sep 28, 2020

nazim1021 commented Oct 30, 2020 •

edited

Loading

williamFalcon commented Oct 31, 2020

edenlightning commented Oct 31, 2020

nazim1021 commented Oct 31, 2020

ref: deprecated results obj, added support for simpler comms (1/n) #3681

ref: deprecated results obj, added support for simpler comms (1/n) #3681

Conversation

williamFalcon commented Sep 27, 2020 • edited Loading

example

Passing around step results.

Hooks and .log are decoupled...

pep8speaks commented Sep 27, 2020 • edited Loading

Comment last updated at 2020-09-28 02:48:30 UTC

mergify bot commented Sep 27, 2020

carmocca Sep 27, 2020 • edited Loading

Choose a reason for hiding this comment

williamFalcon Sep 28, 2020

Choose a reason for hiding this comment

carmocca Sep 28, 2020 • edited Loading

Choose a reason for hiding this comment

Borda Sep 28, 2020

Choose a reason for hiding this comment

mergify bot commented Sep 28, 2020

nazim1021 commented Oct 30, 2020 • edited Loading

williamFalcon commented Oct 31, 2020

edenlightning commented Oct 31, 2020

nazim1021 commented Oct 31, 2020

williamFalcon commented Sep 27, 2020 •

edited

Loading

pep8speaks commented Sep 27, 2020 •

edited

Loading

carmocca Sep 27, 2020 •

edited

Loading

carmocca Sep 28, 2020 •

edited

Loading

nazim1021 commented Oct 30, 2020 •

edited

Loading