From 769a459d27fddf1694aa501ec2ab51c2c4b11590 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrian=20W=C3=A4lchli?= Date: Sun, 17 May 2020 15:14:54 +0200 Subject: [PATCH] remove extra kwargs from Trainer init (#1820) * remove kwargs * remove useless test * rename unknown trainer flag * trainer inheritance and test * blank line * test for unknown arg * changelog --- CHANGELOG.md | 2 ++ pytorch_lightning/trainer/callback_hook.py | 9 +++-- pytorch_lightning/trainer/trainer.py | 2 +- tests/trainer/test_dataloaders.py | 21 +----------- tests/trainer/test_trainer.py | 38 ++++++++++++++++++++++ 5 files changed, 46 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 80f4d820..24bd02fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Fixed `hparam` logging with metrics ([#1647](https://github.com/PyTorchLightning/pytorch-lightning/pull/1647)) +- Fixed an issue with Trainer constructor silently ignoring unkown/misspelled arguments ([#1820](https://github.com/PyTorchLightning/pytorch-lightning/pull/1820)) + ## [0.7.5] - 2020-04-27 ### Changed diff --git a/pytorch_lightning/trainer/callback_hook.py b/pytorch_lightning/trainer/callback_hook.py index 37f56e69..0ba6a54d 100644 --- a/pytorch_lightning/trainer/callback_hook.py +++ b/pytorch_lightning/trainer/callback_hook.py @@ -6,11 +6,10 @@ from pytorch_lightning.callbacks import Callback class TrainerCallbackHookMixin(ABC): - def __init__(self): - # this is just a summary on variables used in this abstract class, - # the proper values/initialisation should be done in child class - self.callbacks: List[Callback] = [] - self.get_model: Callable = ... + # this is just a summary on variables used in this abstract class, + # the proper values/initialisation should be done in child class + callbacks: List[Callback] = [] + get_model: Callable = ... def on_init_start(self): """Called when the trainer initialization begins, model has not yet been set.""" diff --git a/pytorch_lightning/trainer/trainer.py b/pytorch_lightning/trainer/trainer.py index f274dde6..bed2aa05 100644 --- a/pytorch_lightning/trainer/trainer.py +++ b/pytorch_lightning/trainer/trainer.py @@ -142,7 +142,6 @@ class Trainer( use_amp=None, # backward compatible, todo: remove in v0.9.0 show_progress_bar=None, # backward compatible, todo: remove in v0.9.0 nb_sanity_val_steps=None, # backward compatible, todo: remove in v0.8.0 - **kwargs ): r""" @@ -305,6 +304,7 @@ class Trainer( Additionally, can be set to either `power` that estimates the batch size through a power search or `binsearch` that estimates the batch size through a binary search. """ + super().__init__() self.deterministic = deterministic torch.backends.cudnn.deterministic = self.deterministic diff --git a/tests/trainer/test_dataloaders.py b/tests/trainer/test_dataloaders.py index d157768f..f7a19770 100644 --- a/tests/trainer/test_dataloaders.py +++ b/tests/trainer/test_dataloaders.py @@ -289,7 +289,7 @@ def test_inf_train_dataloader(tmpdir, check_interval): trainer = Trainer( default_root_dir=tmpdir, max_epochs=1, - train_check_interval=check_interval, + val_check_interval=check_interval ) result = trainer.fit(model) # verify training completed @@ -315,25 +315,6 @@ def test_inf_val_dataloader(tmpdir, check_interval): assert result == 1 -@pytest.mark.parametrize('check_interval', [50, 1.0]) -def test_inf_test_dataloader(tmpdir, check_interval): - """Test inf test data loader (e.g. IterableDataset)""" - - model = EvalModelTemplate() - model.test_dataloader = model.test_dataloader__infinite - - # logger file to get meta - trainer = Trainer( - default_root_dir=tmpdir, - max_epochs=1, - test_check_interval=check_interval, - ) - result = trainer.fit(model) - - # verify training completed - assert result == 1 - - def test_error_on_zero_len_dataloader(tmpdir): """ Test that error is raised if a zero-length dataloader is defined """ diff --git a/tests/trainer/test_trainer.py b/tests/trainer/test_trainer.py index 969a6be5..0116125a 100644 --- a/tests/trainer/test_trainer.py +++ b/tests/trainer/test_trainer.py @@ -772,3 +772,41 @@ def test_trainer_config(trainer_kwargs, expected): assert trainer.on_gpu is expected["on_gpu"] assert trainer.single_gpu is expected["single_gpu"] assert trainer.num_processes == expected["num_processes"] + + +def test_trainer_subclassing(): + model = EvalModelTemplate() + + # First way of pulling out args from signature is to list them + class TrainerSubclass(Trainer): + + def __init__(self, custom_arg, *args, custom_kwarg='test', **kwargs): + super().__init__(*args, **kwargs) + self.custom_arg = custom_arg + self.custom_kwarg = custom_kwarg + + trainer = TrainerSubclass(123, custom_kwarg='custom', fast_dev_run=True) + result = trainer.fit(model) + assert result == 1 + assert trainer.custom_arg == 123 + assert trainer.custom_kwarg == 'custom' + assert trainer.fast_dev_run + + # Second way is to pop from the dict + # It's a special case because Trainer does not have any positional args + class TrainerSubclass(Trainer): + + def __init__(self, **kwargs): + self.custom_arg = kwargs.pop('custom_arg', 0) + self.custom_kwarg = kwargs.pop('custom_kwarg', 'test') + super().__init__(**kwargs) + + trainer = TrainerSubclass(custom_kwarg='custom', fast_dev_run=True) + result = trainer.fit(model) + assert result == 1 + assert trainer.custom_kwarg == 'custom' + assert trainer.fast_dev_run + + # when we pass in an unknown arg, the base class should complain + with pytest.raises(TypeError, match=r"__init__\(\) got an unexpected keyword argument 'abcdefg'") as e: + TrainerSubclass(abcdefg='unknown_arg')