From f7c799455d3a422fe7e91e058ad0ae9d450cf88a Mon Sep 17 00:00:00 2001 From: Boris Dayma Date: Wed, 9 Dec 2020 09:00:26 -0600 Subject: [PATCH 01/15] feat(wandb): offset logging step when resuming --- pytorch_lightning/loggers/wandb.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pytorch_lightning/loggers/wandb.py b/pytorch_lightning/loggers/wandb.py index 24007c3a04307..5168cae8b921d 100644 --- a/pytorch_lightning/loggers/wandb.py +++ b/pytorch_lightning/loggers/wandb.py @@ -103,7 +103,7 @@ def __init__( self._log_model = log_model self._prefix = prefix self._kwargs = kwargs - # logging multiple Trainer on a single W&B run (k-fold, etc) + # logging multiple Trainer on a single W&B run (k-fold, resuming, etc) self._step_offset = 0 def __getstate__(self): @@ -134,6 +134,8 @@ def experiment(self) -> Run: self._experiment = wandb.init( name=self._name, dir=self._save_dir, project=self._project, anonymous=self._anonymous, id=self._id, resume='allow', **self._kwargs) if wandb.run is None else wandb.run + # offset logging step when resuming a run + self._step_offset = self._experiment.step # save checkpoints in wandb dir to upload on W&B servers if self._log_model: self._save_dir = self._experiment.dir From a93cd0a78fc9bf06dcc6f91885926869acdf1f26 Mon Sep 17 00:00:00 2001 From: Boris Dayma Date: Wed, 9 Dec 2020 09:38:07 -0600 Subject: [PATCH 02/15] feat(wandb): output warnings --- pytorch_lightning/loggers/wandb.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pytorch_lightning/loggers/wandb.py b/pytorch_lightning/loggers/wandb.py index 5168cae8b921d..e669cfa01aae3 100644 --- a/pytorch_lightning/loggers/wandb.py +++ b/pytorch_lightning/loggers/wandb.py @@ -31,6 +31,7 @@ from pytorch_lightning.loggers.base import LightningLoggerBase, rank_zero_experiment from pytorch_lightning.utilities import rank_zero_only +from pytorch_lightning import _logger as log class WandbLogger(LightningLoggerBase): @@ -66,6 +67,8 @@ class WandbLogger(LightningLoggerBase): wandb_logger = WandbLogger() trainer = Trainer(logger=wandb_logger) + Note: When logging manually through `wandb.log` or `trainer.logger.experiment.log`, make sure to use `commit=False` so the logging step does not increase. + See Also: - `Tutorial `__ @@ -156,6 +159,9 @@ def log_metrics(self, metrics: Dict[str, float], step: Optional[int] = None) -> assert rank_zero_only.rank == 0, 'experiment tried to log from global_rank != 0' metrics = self._add_prefix(metrics) + logging_step = step + self._step_offset + if logging_step < self.experiment.step: + log.warning('Trying to log at a previous step. Use `commit=False` if logging metrics manually.') self.experiment.log(metrics, step=(step + self._step_offset) if step is not None else None) @property From d00e57ae951a6fe6587dfbc8444d1bef0fa9bead Mon Sep 17 00:00:00 2001 From: Boris Dayma Date: Wed, 9 Dec 2020 21:02:30 -0600 Subject: [PATCH 03/15] fix(wandb): allow step to be None --- pytorch_lightning/loggers/wandb.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pytorch_lightning/loggers/wandb.py b/pytorch_lightning/loggers/wandb.py index e669cfa01aae3..2ee1a7ffebb98 100644 --- a/pytorch_lightning/loggers/wandb.py +++ b/pytorch_lightning/loggers/wandb.py @@ -159,8 +159,7 @@ def log_metrics(self, metrics: Dict[str, float], step: Optional[int] = None) -> assert rank_zero_only.rank == 0, 'experiment tried to log from global_rank != 0' metrics = self._add_prefix(metrics) - logging_step = step + self._step_offset - if logging_step < self.experiment.step: + if step is not None and step + self._step_offset < self.experiment.step: log.warning('Trying to log at a previous step. Use `commit=False` if logging metrics manually.') self.experiment.log(metrics, step=(step + self._step_offset) if step is not None else None) From dc3794a9abc003ad4ff5211987a779764970b73e Mon Sep 17 00:00:00 2001 From: Boris Dayma Date: Wed, 9 Dec 2020 21:02:46 -0600 Subject: [PATCH 04/15] test(wandb): update tests --- tests/loggers/test_wandb.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/loggers/test_wandb.py b/tests/loggers/test_wandb.py index 33211e6492d91..e9d18d9346eb9 100644 --- a/tests/loggers/test_wandb.py +++ b/tests/loggers/test_wandb.py @@ -34,6 +34,9 @@ def test_wandb_logger_init(wandb): wandb.init.assert_called_once() wandb.init().log.assert_called_once_with({'acc': 1.0}, step=None) + # mock wandb step + wandb.init().step = 0 + # test wandb.init not called if there is a W&B run wandb.init().log.reset_mock() wandb.init.reset_mock() @@ -71,6 +74,7 @@ def test_wandb_pickle(wandb, tmpdir): class Experiment: """ """ id = 'the_id' + step = 0 def project_name(self): return 'the_project_name' @@ -110,6 +114,8 @@ def test_wandb_logger_dirs_creation(wandb, tmpdir): # mock return values of experiment logger.experiment.id = '1' logger.experiment.project_name.return_value = 'project' + logger.experiment.step = 0 + logger._step_offset = 0 for _ in range(2): _ = logger.experiment From eb756769c319196c7cd2192a808a01a560508981 Mon Sep 17 00:00:00 2001 From: Boris Dayma Date: Wed, 9 Dec 2020 21:08:05 -0600 Subject: [PATCH 05/15] feat(wandb): display warning only once --- pytorch_lightning/loggers/wandb.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pytorch_lightning/loggers/wandb.py b/pytorch_lightning/loggers/wandb.py index 2ee1a7ffebb98..b3c0292bc25b4 100644 --- a/pytorch_lightning/loggers/wandb.py +++ b/pytorch_lightning/loggers/wandb.py @@ -77,6 +77,7 @@ class WandbLogger(LightningLoggerBase): """ LOGGER_JOIN_CHAR = '-' + WARNING_DISPLAYED = False def __init__( self, @@ -159,8 +160,9 @@ def log_metrics(self, metrics: Dict[str, float], step: Optional[int] = None) -> assert rank_zero_only.rank == 0, 'experiment tried to log from global_rank != 0' metrics = self._add_prefix(metrics) - if step is not None and step + self._step_offset < self.experiment.step: + if not WandbLogger.WARNING_DISPLAYED and (step is not None and step + self._step_offset < self.experiment.step): log.warning('Trying to log at a previous step. Use `commit=False` if logging metrics manually.') + WandbLogger.WARNING_DISPLAYED = True self.experiment.log(metrics, step=(step + self._step_offset) if step is not None else None) @property From 69bf4bb1c17a42e4bdc2258713f851623280c667 Mon Sep 17 00:00:00 2001 From: Boris Dayma Date: Wed, 9 Dec 2020 21:15:52 -0600 Subject: [PATCH 06/15] style: fix PEP issues --- pytorch_lightning/loggers/wandb.py | 3 ++- tests/loggers/test_wandb.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pytorch_lightning/loggers/wandb.py b/pytorch_lightning/loggers/wandb.py index b3c0292bc25b4..3d0ec9a2e217e 100644 --- a/pytorch_lightning/loggers/wandb.py +++ b/pytorch_lightning/loggers/wandb.py @@ -67,7 +67,8 @@ class WandbLogger(LightningLoggerBase): wandb_logger = WandbLogger() trainer = Trainer(logger=wandb_logger) - Note: When logging manually through `wandb.log` or `trainer.logger.experiment.log`, make sure to use `commit=False` so the logging step does not increase. + Note: When logging manually through `wandb.log` or `trainer.logger.experiment.log`, + make sure to use `commit=False` so the logging step does not increase. See Also: - `Tutorial Date: Wed, 9 Dec 2020 21:42:32 -0600 Subject: [PATCH 07/15] tests(wandb): fix tests --- tests/loggers/test_all.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/loggers/test_all.py b/tests/loggers/test_all.py index ba5791c7b9f4a..7072bf0ae47e7 100644 --- a/tests/loggers/test_all.py +++ b/tests/loggers/test_all.py @@ -74,7 +74,9 @@ def test_loggers_fit_test_all(tmpdir, monkeypatch): with mock.patch('pytorch_lightning.loggers.test_tube.Experiment'): _test_loggers_fit_test(tmpdir, TestTubeLogger) - with mock.patch('pytorch_lightning.loggers.wandb.wandb'): + with mock.patch('pytorch_lightning.loggers.wandb.wandb') as wandb: + wandb.run = None + wandb.init().step = 0 _test_loggers_fit_test(tmpdir, WandbLogger) @@ -366,7 +368,9 @@ def test_logger_with_prefix_all(tmpdir, monkeypatch): logger.experiment.log.assert_called_once_with({"tmp-test": 1.0}, global_step=0) # WandB - with mock.patch('pytorch_lightning.loggers.wandb.wandb') as wandb: + with mock.patch('pytorch_lightning.loggers.wandb.wandb') as wandb: logger = _instantiate_logger(WandbLogger, save_idr=tmpdir, prefix=prefix) + wandb.run = None + wandb.init().step = 0 logger.log_metrics({"test": 1.0}, step=0) logger.experiment.log.assert_called_once_with({'tmp-test': 1.0}, step=0) From dbc15f22518dd15ee5b628c166fb2bcf790bde2c Mon Sep 17 00:00:00 2001 From: Boris Dayma Date: Wed, 9 Dec 2020 21:44:46 -0600 Subject: [PATCH 08/15] tests(wandb): improve test --- tests/loggers/test_wandb.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/loggers/test_wandb.py b/tests/loggers/test_wandb.py index 77cd9cc2f7b7f..708f15cfb2734 100644 --- a/tests/loggers/test_wandb.py +++ b/tests/loggers/test_wandb.py @@ -112,10 +112,11 @@ def test_wandb_logger_dirs_creation(wandb, tmpdir): assert logger.name is None # mock return values of experiment + wandb.run = None + wandb.init().step = 0 logger.experiment.id = '1' logger.experiment.project_name.return_value = 'project' logger.experiment.step = 0 - logger._step_offset = 0 for _ in range(2): _ = logger.experiment From ea4f88e496da324270b435f9214dfb1be705af8c Mon Sep 17 00:00:00 2001 From: Boris Dayma Date: Wed, 9 Dec 2020 21:48:23 -0600 Subject: [PATCH 09/15] style: fix whitespace --- tests/loggers/test_all.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/loggers/test_all.py b/tests/loggers/test_all.py index 7072bf0ae47e7..89c731d432ee9 100644 --- a/tests/loggers/test_all.py +++ b/tests/loggers/test_all.py @@ -368,7 +368,7 @@ def test_logger_with_prefix_all(tmpdir, monkeypatch): logger.experiment.log.assert_called_once_with({"tmp-test": 1.0}, global_step=0) # WandB - with mock.patch('pytorch_lightning.loggers.wandb.wandb') as wandb: + with mock.patch('pytorch_lightning.loggers.wandb.wandb') as wandb: logger = _instantiate_logger(WandbLogger, save_idr=tmpdir, prefix=prefix) wandb.run = None wandb.init().step = 0 From 1c3bc82dd6ee6d1d765532f1fd9462260ccd2d59 Mon Sep 17 00:00:00 2001 From: Boris Dayma Date: Wed, 9 Dec 2020 23:10:03 -0600 Subject: [PATCH 10/15] feat: improve warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Adrian Wälchli --- pytorch_lightning/loggers/wandb.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pytorch_lightning/loggers/wandb.py b/pytorch_lightning/loggers/wandb.py index 3d0ec9a2e217e..7b3cf26403717 100644 --- a/pytorch_lightning/loggers/wandb.py +++ b/pytorch_lightning/loggers/wandb.py @@ -162,7 +162,7 @@ def log_metrics(self, metrics: Dict[str, float], step: Optional[int] = None) -> metrics = self._add_prefix(metrics) if not WandbLogger.WARNING_DISPLAYED and (step is not None and step + self._step_offset < self.experiment.step): - log.warning('Trying to log at a previous step. Use `commit=False` if logging metrics manually.') + log.warning('Trying to log at a previous step. Use `commit=False` when logging metrics manually.') WandbLogger.WARNING_DISPLAYED = True self.experiment.log(metrics, step=(step + self._step_offset) if step is not None else None) From 6c02a2cbc8c37f09b866f52aa67fd9dd8fa0b55b Mon Sep 17 00:00:00 2001 From: Boris Dayma Date: Thu, 10 Dec 2020 10:33:27 -0600 Subject: [PATCH 11/15] feat(wandb): use variable from class instance Co-authored-by: Jirka Borovec --- pytorch_lightning/loggers/wandb.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pytorch_lightning/loggers/wandb.py b/pytorch_lightning/loggers/wandb.py index 7b3cf26403717..2eaba2cd680c8 100644 --- a/pytorch_lightning/loggers/wandb.py +++ b/pytorch_lightning/loggers/wandb.py @@ -161,9 +161,9 @@ def log_metrics(self, metrics: Dict[str, float], step: Optional[int] = None) -> assert rank_zero_only.rank == 0, 'experiment tried to log from global_rank != 0' metrics = self._add_prefix(metrics) - if not WandbLogger.WARNING_DISPLAYED and (step is not None and step + self._step_offset < self.experiment.step): + if not self.WARNING_DISPLAYED and (step is not None and step + self._step_offset < self.experiment.step): log.warning('Trying to log at a previous step. Use `commit=False` when logging metrics manually.') - WandbLogger.WARNING_DISPLAYED = True + self.WARNING_DISPLAYED = True self.experiment.log(metrics, step=(step + self._step_offset) if step is not None else None) @property From 63dcd70499bf18614346fb1ff24565b0ab070418 Mon Sep 17 00:00:00 2001 From: Boris Dayma Date: Thu, 10 Dec 2020 19:23:09 -0600 Subject: [PATCH 12/15] tests(wandb): check warnings --- tests/loggers/test_wandb.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/loggers/test_wandb.py b/tests/loggers/test_wandb.py index 708f15cfb2734..c3edd600c9125 100644 --- a/tests/loggers/test_wandb.py +++ b/tests/loggers/test_wandb.py @@ -23,7 +23,7 @@ @mock.patch('pytorch_lightning.loggers.wandb.wandb') -def test_wandb_logger_init(wandb): +def test_wandb_logger_init(wandb, caplog): """Verify that basic functionality of wandb logger works. Wandb doesn't work well with pytest so we have to mock it out here.""" @@ -52,15 +52,30 @@ def test_wandb_logger_init(wandb): logger.log_metrics({'acc': 1.0}, step=3) wandb.init().log.assert_called_with({'acc': 1.0}, step=6) + # log hyper parameters logger.log_hyperparams({'test': None, 'nested': {'a': 1}, 'b': [2, 3, 4]}) wandb.init().config.update.assert_called_once_with( {'test': 'None', 'nested/a': 1, 'b': [2, 3, 4]}, allow_val_change=True, ) + # watch a model logger.watch('model', 'log', 10) wandb.init().watch.assert_called_once_with('model', log='log', log_freq=10) + # verify warning for logging at a previous step + assert 'Trying to log at a previous step' not in caplog.text + caplog.clear() + # current step from wandb should be 6 (last logged step) + logger.experiment.step = 6 + # logging at step 2 should raise a warning (step_offset is still 3) + logger.log_metrics({'acc': 1.0}, step=2) + assert 'Trying to log at a previous step' in caplog.text + caplog.clear() + # logging again at step 2 should not display again the same warning + logger.log_metrics({'acc': 1.0}, step=2) + assert 'Trying to log at a previous step' not in caplog.text + assert logger.name == wandb.init().project_name() assert logger.version == wandb.init().id From 61cb1998009b1cf8c3554fd40e3e93c9d50a1f31 Mon Sep 17 00:00:00 2001 From: Boris Dayma Date: Mon, 14 Dec 2020 13:49:36 -0600 Subject: [PATCH 13/15] feat(wandb): use WarningCache --- pytorch_lightning/loggers/wandb.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pytorch_lightning/loggers/wandb.py b/pytorch_lightning/loggers/wandb.py index 2eaba2cd680c8..529a2f4cddc77 100644 --- a/pytorch_lightning/loggers/wandb.py +++ b/pytorch_lightning/loggers/wandb.py @@ -31,7 +31,7 @@ from pytorch_lightning.loggers.base import LightningLoggerBase, rank_zero_experiment from pytorch_lightning.utilities import rank_zero_only -from pytorch_lightning import _logger as log +from pytorch_lightning.utilities.warning_utils import WarningCache class WandbLogger(LightningLoggerBase): @@ -78,7 +78,6 @@ class WandbLogger(LightningLoggerBase): """ LOGGER_JOIN_CHAR = '-' - WARNING_DISPLAYED = False def __init__( self, @@ -110,6 +109,7 @@ def __init__( self._kwargs = kwargs # logging multiple Trainer on a single W&B run (k-fold, resuming, etc) self._step_offset = 0 + self.warning_cache = WarningCache() def __getstate__(self): state = self.__dict__.copy() @@ -161,9 +161,8 @@ def log_metrics(self, metrics: Dict[str, float], step: Optional[int] = None) -> assert rank_zero_only.rank == 0, 'experiment tried to log from global_rank != 0' metrics = self._add_prefix(metrics) - if not self.WARNING_DISPLAYED and (step is not None and step + self._step_offset < self.experiment.step): - log.warning('Trying to log at a previous step. Use `commit=False` when logging metrics manually.') - self.WARNING_DISPLAYED = True + if step is not None and step + self._step_offset < self.experiment.step: + self.warning_cache.warn('Trying to log at a previous step. Use `commit=False` when logging metrics manually.') self.experiment.log(metrics, step=(step + self._step_offset) if step is not None else None) @property From f7d832e1854a2c1a4fd69f77a0f7bd9cf93d0844 Mon Sep 17 00:00:00 2001 From: Boris Dayma Date: Mon, 14 Dec 2020 18:43:51 -0600 Subject: [PATCH 14/15] tests(wandb): fix tests --- tests/loggers/test_wandb.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/loggers/test_wandb.py b/tests/loggers/test_wandb.py index c3edd600c9125..135200551d43e 100644 --- a/tests/loggers/test_wandb.py +++ b/tests/loggers/test_wandb.py @@ -22,8 +22,13 @@ from tests.base import EvalModelTemplate, BoringModel +def get_warnings(recwarn): + warnings_text = '\n'.join(str(w.message) for w in recwarn.list) + recwarn.clear() + return warnings_text + @mock.patch('pytorch_lightning.loggers.wandb.wandb') -def test_wandb_logger_init(wandb, caplog): +def test_wandb_logger_init(wandb, recwarn): """Verify that basic functionality of wandb logger works. Wandb doesn't work well with pytest so we have to mock it out here.""" @@ -64,17 +69,15 @@ def test_wandb_logger_init(wandb, caplog): wandb.init().watch.assert_called_once_with('model', log='log', log_freq=10) # verify warning for logging at a previous step - assert 'Trying to log at a previous step' not in caplog.text - caplog.clear() + assert 'Trying to log at a previous step' not in get_warnings(recwarn) # current step from wandb should be 6 (last logged step) logger.experiment.step = 6 # logging at step 2 should raise a warning (step_offset is still 3) logger.log_metrics({'acc': 1.0}, step=2) - assert 'Trying to log at a previous step' in caplog.text - caplog.clear() + assert 'Trying to log at a previous step' in get_warnings(recwarn) # logging again at step 2 should not display again the same warning logger.log_metrics({'acc': 1.0}, step=2) - assert 'Trying to log at a previous step' not in caplog.text + assert 'Trying to log at a previous step' not in get_warnings(recwarn) assert logger.name == wandb.init().project_name() assert logger.version == wandb.init().id From d4d500e38a0d18441d72e6ae2ab387d4989b5709 Mon Sep 17 00:00:00 2001 From: Boris Dayma Date: Mon, 14 Dec 2020 18:47:29 -0600 Subject: [PATCH 15/15] style: fix formatting --- tests/loggers/test_wandb.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/loggers/test_wandb.py b/tests/loggers/test_wandb.py index 135200551d43e..a44b19ca39270 100644 --- a/tests/loggers/test_wandb.py +++ b/tests/loggers/test_wandb.py @@ -27,6 +27,7 @@ def get_warnings(recwarn): recwarn.clear() return warnings_text + @mock.patch('pytorch_lightning.loggers.wandb.wandb') def test_wandb_logger_init(wandb, recwarn): """Verify that basic functionality of wandb logger works.