From fe48785724ef9a5b7c64ebea70bc40d702315929 Mon Sep 17 00:00:00 2001 From: ivannz Date: Mon, 25 May 2020 17:00:59 +0300 Subject: [PATCH 1/5] fixed undesired behaviour due to dict.fromkeys --- pytorch_lightning/callbacks/lr_logger.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pytorch_lightning/callbacks/lr_logger.py b/pytorch_lightning/callbacks/lr_logger.py index c8aab75b87089..15967bc91d8b4 100755 --- a/pytorch_lightning/callbacks/lr_logger.py +++ b/pytorch_lightning/callbacks/lr_logger.py @@ -59,7 +59,7 @@ def on_train_start(self, trainer, pl_module): names = self._find_names(trainer.lr_schedulers) # Initialize for storing values - self.lrs = dict.fromkeys(names, []) + self.lrs = {name: [] for name in names} def on_batch_start(self, trainer, pl_module): latest_stat = self._extract_lr(trainer, 'step') From b7b123aa643402a641eae754b00a48cd61d42955 Mon Sep 17 00:00:00 2001 From: ivannz Date: Mon, 25 May 2020 18:03:02 +0300 Subject: [PATCH 2/5] a test for log length consistency --- tests/callbacks/test_callbacks.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/callbacks/test_callbacks.py b/tests/callbacks/test_callbacks.py index 57b0b537b4dca..b5bc800a1e690 100644 --- a/tests/callbacks/test_callbacks.py +++ b/tests/callbacks/test_callbacks.py @@ -318,7 +318,7 @@ def test_lr_logger_multi_lrs(tmpdir): lr_logger = LearningRateLogger() trainer = Trainer( default_root_dir=tmpdir, - max_epochs=1, + max_epochs=10, val_percent_check=0.1, train_percent_check=0.5, callbacks=[lr_logger] @@ -331,6 +331,8 @@ def test_lr_logger_multi_lrs(tmpdir): 'Number of learning rates logged does not match number of lr schedulers' assert all([k in ['lr-Adam', 'lr-Adam-1'] for k in lr_logger.lrs.keys()]), \ 'Names of learning rates not set correctly' + assert all(len(lr) == trainer.max_epochs for k, lr in lr_logger.lrs.items()), \ + 'Length of logged learning rates exceeds the number of epochs' def test_lr_logger_param_groups(tmpdir): From 1b80db14539799c38011497b2c3fadfb478ec369 Mon Sep 17 00:00:00 2001 From: ivannz Date: Mon, 25 May 2020 18:26:53 +0300 Subject: [PATCH 3/5] runtime-warn if no schedulers are configured --- pytorch_lightning/callbacks/lr_logger.py | 15 +++++++++------ tests/callbacks/test_callbacks.py | 20 ++++++++++++++++++++ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/pytorch_lightning/callbacks/lr_logger.py b/pytorch_lightning/callbacks/lr_logger.py index 15967bc91d8b4..43fee3477abf9 100755 --- a/pytorch_lightning/callbacks/lr_logger.py +++ b/pytorch_lightning/callbacks/lr_logger.py @@ -10,6 +10,8 @@ from pytorch_lightning.callbacks.base import Callback from pytorch_lightning.utilities.exceptions import MisconfigurationException +from pytorch_lightning.utilities import rank_zero_warn + class LearningRateLogger(Callback): r""" @@ -45,16 +47,17 @@ def on_train_start(self, trainer, pl_module): schedulers in the case of multiple of the same type or in the case of multiple parameter groups """ - if trainer.lr_schedulers == []: - raise MisconfigurationException( - 'Cannot use LearningRateLogger callback with models that have no' - ' learning rate schedulers. Please see documentation for' - ' `configure_optimizers` method.') - if not trainer.logger: raise MisconfigurationException( 'Cannot use LearningRateLogger callback with Trainer that has no logger.') + if not trainer.lr_schedulers: + rank_zero_warn( + 'You are using LearningRateLogger callback with models that' + ' have no learning rate schedulers. Please see documentation' + ' for `configure_optimizers` method.', RuntimeWarning + ) + # Find names for schedulers names = self._find_names(trainer.lr_schedulers) diff --git a/tests/callbacks/test_callbacks.py b/tests/callbacks/test_callbacks.py index b5bc800a1e690..5bd4f24659bf6 100644 --- a/tests/callbacks/test_callbacks.py +++ b/tests/callbacks/test_callbacks.py @@ -3,6 +3,8 @@ import tests.base.utils as tutils from pytorch_lightning import Callback from pytorch_lightning import Trainer, LightningModule +from pytorch_lightning.utilities.exceptions import MisconfigurationException + from pytorch_lightning.callbacks import EarlyStopping, LearningRateLogger, ModelCheckpoint from pytorch_lightning.loggers import TensorBoardLogger from tests.base import EvalModelTemplate @@ -308,6 +310,24 @@ def test_lr_logger_single_lr(tmpdir): 'Names of learning rates not set correctly' +def test_lr_logger_no_lr(tmpdir): + tutils.reset_seed() + + model = EvalModelTemplate() + + lr_logger = LearningRateLogger() + trainer = Trainer( + default_root_dir=tmpdir, + max_epochs=5, + val_percent_check=0.1, + train_percent_check=0.5, + callbacks=[lr_logger] + ) + + with pytest.warns(RuntimeWarning): + results = trainer.fit(model) + + def test_lr_logger_multi_lrs(tmpdir): """ Test that learning rates are extracted and logged for multi lr schedulers """ tutils.reset_seed() From f6bebc1cab805194df75cc8e00802e4900586487 Mon Sep 17 00:00:00 2001 From: Jirka Date: Mon, 25 May 2020 21:54:29 +0200 Subject: [PATCH 4/5] chlog --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6280a1beeab6..73c5972a44cfe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,7 +40,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Fixed an issue with `Trainer.from_argparse_args` when passing in unknown Trainer args ([#1932](https://github.com/PyTorchLightning/pytorch-lightning/pull/1932)) -- Fix bug related to logger not being reset correctly for model after tuner algorithms ([#1933](https://github.com/PyTorchLightning/pytorch-lightning/pull/1933)) +- Fixed bug related to logger not being reset correctly for model after tuner algorithms ([#1933](https://github.com/PyTorchLightning/pytorch-lightning/pull/1933)) + +- Fixed `LearningRateLogger` in multi-scheduler setting ([#1944](https://github.com/PyTorchLightning/pytorch-lightning/pull/1944)) ## [0.7.6] - 2020-05-16 From b4f785588d96e67a44d552e99e76c37296cd568a Mon Sep 17 00:00:00 2001 From: Jirka Date: Mon, 25 May 2020 21:57:52 +0200 Subject: [PATCH 5/5] move --- tests/callbacks/test_callbacks.py | 101 +---------------------------- tests/callbacks/test_lr.py | 102 ++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 98 deletions(-) create mode 100644 tests/callbacks/test_lr.py diff --git a/tests/callbacks/test_callbacks.py b/tests/callbacks/test_callbacks.py index 5bd4f24659bf6..c962e6cdb0a55 100644 --- a/tests/callbacks/test_callbacks.py +++ b/tests/callbacks/test_callbacks.py @@ -1,14 +1,13 @@ +from pathlib import Path + import pytest import tests.base.utils as tutils from pytorch_lightning import Callback from pytorch_lightning import Trainer, LightningModule -from pytorch_lightning.utilities.exceptions import MisconfigurationException - -from pytorch_lightning.callbacks import EarlyStopping, LearningRateLogger, ModelCheckpoint +from pytorch_lightning.callbacks import EarlyStopping, ModelCheckpoint from pytorch_lightning.loggers import TensorBoardLogger from tests.base import EvalModelTemplate -from pathlib import Path def test_trainer_callback_system(tmpdir): @@ -283,97 +282,3 @@ def test_model_checkpoint_path(tmpdir, logger_version, expected): ckpt_version = Path(trainer.ckpt_path).parent.name assert ckpt_version == expected - - -def test_lr_logger_single_lr(tmpdir): - """ Test that learning rates are extracted and logged for single lr scheduler""" - tutils.reset_seed() - - model = EvalModelTemplate() - model.configure_optimizers = model.configure_optimizers__single_scheduler - - lr_logger = LearningRateLogger() - trainer = Trainer( - default_root_dir=tmpdir, - max_epochs=5, - val_percent_check=0.1, - train_percent_check=0.5, - callbacks=[lr_logger] - ) - results = trainer.fit(model) - - assert results == 1 - assert lr_logger.lrs, 'No learning rates logged' - assert len(lr_logger.lrs) == len(trainer.lr_schedulers), \ - 'Number of learning rates logged does not match number of lr schedulers' - assert all([k in ['lr-Adam'] for k in lr_logger.lrs.keys()]), \ - 'Names of learning rates not set correctly' - - -def test_lr_logger_no_lr(tmpdir): - tutils.reset_seed() - - model = EvalModelTemplate() - - lr_logger = LearningRateLogger() - trainer = Trainer( - default_root_dir=tmpdir, - max_epochs=5, - val_percent_check=0.1, - train_percent_check=0.5, - callbacks=[lr_logger] - ) - - with pytest.warns(RuntimeWarning): - results = trainer.fit(model) - - -def test_lr_logger_multi_lrs(tmpdir): - """ Test that learning rates are extracted and logged for multi lr schedulers """ - tutils.reset_seed() - - model = EvalModelTemplate() - model.configure_optimizers = model.configure_optimizers__multiple_schedulers - - lr_logger = LearningRateLogger() - trainer = Trainer( - default_root_dir=tmpdir, - max_epochs=10, - val_percent_check=0.1, - train_percent_check=0.5, - callbacks=[lr_logger] - ) - results = trainer.fit(model) - - assert results == 1 - assert lr_logger.lrs, 'No learning rates logged' - assert len(lr_logger.lrs) == len(trainer.lr_schedulers), \ - 'Number of learning rates logged does not match number of lr schedulers' - assert all([k in ['lr-Adam', 'lr-Adam-1'] for k in lr_logger.lrs.keys()]), \ - 'Names of learning rates not set correctly' - assert all(len(lr) == trainer.max_epochs for k, lr in lr_logger.lrs.items()), \ - 'Length of logged learning rates exceeds the number of epochs' - - -def test_lr_logger_param_groups(tmpdir): - """ Test that learning rates are extracted and logged for single lr scheduler""" - tutils.reset_seed() - - model = EvalModelTemplate() - model.configure_optimizers = model.configure_optimizers__param_groups - - lr_logger = LearningRateLogger() - trainer = Trainer( - default_root_dir=tmpdir, - max_epochs=5, - val_percent_check=0.1, - train_percent_check=0.5, - callbacks=[lr_logger] - ) - results = trainer.fit(model) - - assert lr_logger.lrs, 'No learning rates logged' - assert len(lr_logger.lrs) == 2 * len(trainer.lr_schedulers), \ - 'Number of learning rates logged does not match number of param groups' - assert all([k in ['lr-Adam/pg1', 'lr-Adam/pg2'] for k in lr_logger.lrs.keys()]), \ - 'Names of learning rates not set correctly' diff --git a/tests/callbacks/test_lr.py b/tests/callbacks/test_lr.py new file mode 100644 index 0000000000000..e8c914ef2a084 --- /dev/null +++ b/tests/callbacks/test_lr.py @@ -0,0 +1,102 @@ +import pytest + +import tests.base.utils as tutils +from pytorch_lightning import Trainer +from pytorch_lightning.callbacks import LearningRateLogger +from tests.base import EvalModelTemplate + + +def test_lr_logger_single_lr(tmpdir): + """ Test that learning rates are extracted and logged for single lr scheduler. """ + tutils.reset_seed() + + model = EvalModelTemplate() + model.configure_optimizers = model.configure_optimizers__single_scheduler + + lr_logger = LearningRateLogger() + trainer = Trainer( + default_root_dir=tmpdir, + max_epochs=5, + val_percent_check=0.1, + train_percent_check=0.5, + callbacks=[lr_logger] + ) + result = trainer.fit(model) + assert result + + assert lr_logger.lrs, 'No learning rates logged' + assert len(lr_logger.lrs) == len(trainer.lr_schedulers), \ + 'Number of learning rates logged does not match number of lr schedulers' + assert all([k in ['lr-Adam'] for k in lr_logger.lrs.keys()]), \ + 'Names of learning rates not set correctly' + + +def test_lr_logger_no_lr(tmpdir): + tutils.reset_seed() + + model = EvalModelTemplate() + + lr_logger = LearningRateLogger() + trainer = Trainer( + default_root_dir=tmpdir, + max_epochs=5, + val_percent_check=0.1, + train_percent_check=0.5, + callbacks=[lr_logger] + ) + + with pytest.warns(RuntimeWarning): + result = trainer.fit(model) + assert result + + +def test_lr_logger_multi_lrs(tmpdir): + """ Test that learning rates are extracted and logged for multi lr schedulers. """ + tutils.reset_seed() + + model = EvalModelTemplate() + model.configure_optimizers = model.configure_optimizers__multiple_schedulers + + lr_logger = LearningRateLogger() + trainer = Trainer( + default_root_dir=tmpdir, + max_epochs=10, + val_percent_check=0.1, + train_percent_check=0.5, + callbacks=[lr_logger] + ) + result = trainer.fit(model) + assert result + + assert lr_logger.lrs, 'No learning rates logged' + assert len(lr_logger.lrs) == len(trainer.lr_schedulers), \ + 'Number of learning rates logged does not match number of lr schedulers' + assert all([k in ['lr-Adam', 'lr-Adam-1'] for k in lr_logger.lrs.keys()]), \ + 'Names of learning rates not set correctly' + assert all(len(lr) == trainer.max_epochs for k, lr in lr_logger.lrs.items()), \ + 'Length of logged learning rates exceeds the number of epochs' + + +def test_lr_logger_param_groups(tmpdir): + """ Test that learning rates are extracted and logged for single lr scheduler. """ + tutils.reset_seed() + + model = EvalModelTemplate() + model.configure_optimizers = model.configure_optimizers__param_groups + + lr_logger = LearningRateLogger() + trainer = Trainer( + default_root_dir=tmpdir, + max_epochs=5, + val_percent_check=0.1, + train_percent_check=0.5, + callbacks=[lr_logger] + ) + result = trainer.fit(model) + assert result + + assert lr_logger.lrs, 'No learning rates logged' + assert len(lr_logger.lrs) == 2 * len(trainer.lr_schedulers), \ + 'Number of learning rates logged does not match number of param groups' + assert all([k in ['lr-Adam/pg1', 'lr-Adam/pg2'] for k in lr_logger.lrs.keys()]), \ + 'Names of learning rates not set correctly'