From 7b917de94642f63eedaffde79fb973705d2288dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrian=20W=C3=A4lchli?= Date: Thu, 20 Aug 2020 01:01:55 +0200 Subject: [PATCH] fix setting batch_size attribute in batch_size finder (finishing PR #2523) (#3043) * lightning attr fix * revert refactor * create test * separate test * changelog update * tests * revert * Update pytorch_lightning/trainer/training_tricks.py Co-authored-by: William Falcon --- CHANGELOG.md | 3 ++ pytorch_lightning/trainer/training_tricks.py | 29 +++++------ tests/trainer/test_trainer_tricks.py | 55 +++++++++++++++----- 3 files changed, 60 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index abd870c1f9..5ff11f5822 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -148,8 +148,11 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Fixed gathering of results with tensors of varying shape ([#3020](https://github.com/PyTorchLightning/pytorch-lightning/pull/3020)) +- Fixed batch size auto-scaling feature to set the new value on the correct model attribute ([#3043](https://github.com/PyTorchLightning/pytorch-lightning/pull/3043)) + - Fixed automatic batch scaling not working with half precision ([#3045](https://github.com/PyTorchLightning/pytorch-lightning/pull/3045)) + ## [0.8.5] - 2020-07-09 ### Added diff --git a/pytorch_lightning/trainer/training_tricks.py b/pytorch_lightning/trainer/training_tricks.py index a124dd9aab..04d77916d7 100644 --- a/pytorch_lightning/trainer/training_tricks.py +++ b/pytorch_lightning/trainer/training_tricks.py @@ -24,9 +24,10 @@ from pytorch_lightning import _logger as log from pytorch_lightning.callbacks import GradientAccumulationScheduler from pytorch_lightning.core.lightning import LightningModule from pytorch_lightning.loggers.base import DummyLogger -from pytorch_lightning.utilities import AMPType +from pytorch_lightning.utilities import AMPType, rank_zero_warn from pytorch_lightning.utilities.exceptions import MisconfigurationException from pytorch_lightning.utilities.memory import is_oom_error, garbage_collection_cuda +from pytorch_lightning.utilities.parsing import lightning_hasattr, lightning_getattr, lightning_setattr try: from apex import amp @@ -158,11 +159,15 @@ class TrainerTrainingTricksMixin(ABC): algorithm is terminated """ - if not hasattr(model, batch_arg_name): - if not hasattr(model.hparams, batch_arg_name): - raise MisconfigurationException( - 'Neither of `model.batch_size` and `model.hparams.batch_size` found.' - ) + if not lightning_hasattr(model, batch_arg_name): + raise MisconfigurationException( + f'Field {batch_arg_name} not found in both `model` and `model.hparams`') + if hasattr(model, batch_arg_name) and hasattr(model, "hparams") and batch_arg_name in model.hparams: + rank_zero_warn( + f'Field `model.{batch_arg_name}` and `model.hparams.{batch_arg_name}` are mutually exclusive!' + f' `model.{batch_arg_name}` will be used as the initial batch size for scaling.' + f' If this is not the intended behavior, please remove either one.' + ) if hasattr(model.train_dataloader, 'patch_loader_code'): raise MisconfigurationException('The batch scaling feature cannot be used with dataloaders' @@ -268,15 +273,9 @@ def _adjust_batch_size(trainer, """ model = trainer.get_model() - if hasattr(model, batch_arg_name): - batch_size = getattr(model, batch_arg_name) - else: - batch_size = getattr(model.hparams, batch_arg_name) + batch_size = lightning_getattr(model, batch_arg_name) if value: - if hasattr(model, batch_arg_name): - setattr(model, batch_arg_name, value) - else: - setattr(model.hparams, batch_arg_name, value) + lightning_setattr(model, batch_arg_name, value) new_size = value if desc: log.info(f'Batch size {batch_size} {desc}, trying batch size {new_size}') @@ -284,7 +283,7 @@ def _adjust_batch_size(trainer, new_size = int(batch_size * factor) if desc: log.info(f'Batch size {batch_size} {desc}, trying batch size {new_size}') - setattr(model.hparams, batch_arg_name, new_size) + lightning_setattr(model, batch_arg_name, new_size) return new_size diff --git a/tests/trainer/test_trainer_tricks.py b/tests/trainer/test_trainer_tricks.py index aca8a458ac..66f5b72001 100755 --- a/tests/trainer/test_trainer_tricks.py +++ b/tests/trainer/test_trainer_tricks.py @@ -196,28 +196,59 @@ def test_trainer_reset_correctly(tmpdir): f'Attribute {key} was not reset correctly after learning rate finder' -@pytest.mark.parametrize('scale_arg', ['power', 'binsearch']) -def test_trainer_arg(tmpdir, scale_arg): - """ Check that trainer arg works with bool input. """ +@pytest.mark.parametrize('scale_arg', ['power', 'binsearch', True]) +def test_auto_scale_batch_size_trainer_arg(tmpdir, scale_arg): + """ Test possible values for 'batch size auto scaling' Trainer argument. """ tutils.reset_seed() - hparams = EvalModelTemplate.get_default_hparams() model = EvalModelTemplate(**hparams) - before_batch_size = hparams.get('batch_size') - # logger file to get meta - trainer = Trainer( - default_root_dir=tmpdir, - max_epochs=1, - auto_scale_batch_size=scale_arg, - ) - + trainer = Trainer(default_root_dir=tmpdir, max_epochs=1, auto_scale_batch_size=scale_arg) trainer.fit(model) after_batch_size = model.batch_size assert before_batch_size != after_batch_size, \ 'Batch size was not altered after running auto scaling of batch size' +@pytest.mark.parametrize('use_hparams', [True, False]) +def test_auto_scale_batch_size_set_model_attribute(tmpdir, use_hparams): + """ Test that new batch size gets written to the correct hyperparameter attribute. """ + tutils.reset_seed() + + hparams = EvalModelTemplate.get_default_hparams() + before_batch_size = hparams.get('batch_size') + + class HparamsEvalModelTemplate(EvalModelTemplate): + + def dataloader(self, *args, **kwargs): + # artificially set batch_size so we can get a dataloader + # remove it immediately after, because we want only self.hparams.batch_size + setattr(self, "batch_size", before_batch_size) + dataloader = super().dataloader(*args, **kwargs) + del self.batch_size + return dataloader + + model_class = HparamsEvalModelTemplate if use_hparams else EvalModelTemplate + model = model_class(**hparams) + + trainer = Trainer(default_root_dir=tmpdir, max_epochs=1, auto_scale_batch_size=True) + trainer.fit(model) + after_batch_size = model.hparams.batch_size if use_hparams else model.batch_size + assert before_batch_size != after_batch_size + + +def test_auto_scale_batch_size_duplicate_attribute_warning(tmpdir): + """ Test for a warning when model.batch_size and model.hparams.batch_size both present. """ + hparams = EvalModelTemplate.get_default_hparams() + model = EvalModelTemplate(**hparams) + model.hparams = hparams + # now we have model.batch_size and model.hparams.batch_size + trainer = Trainer(default_root_dir=tmpdir, max_steps=1, auto_scale_batch_size=True) + expected_message = "Field `model.batch_size` and `model.hparams.batch_size` are mutually exclusive!" + with pytest.warns(UserWarning, match=expected_message): + trainer.fit(model) + + @pytest.mark.parametrize('scale_method', ['power', 'binsearch']) def test_call_to_trainer_method(tmpdir, scale_method): """ Test that calling the trainer method itself works. """