From f895e9f0a811d11c54308568f1c10fc0d56c8a02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrian=20W=C3=A4lchli?= Date: Fri, 9 Apr 2021 10:20:43 +0100 Subject: [PATCH] fix gpus default for Trainer.add_argparse_args (#6898) (cherry picked from commit 9c9e2a0325a6a5d25f0f82859965ffd330ea1acd) --- CHANGELOG.md | 23 ++++++++----------- pytorch_lightning/utilities/argparse.py | 8 ------- pytorch_lightning/utilities/device_parser.py | 9 -------- tests/loggers/test_wandb.py | 4 +--- tests/trainer/test_trainer_cli.py | 24 ++++++++++---------- tests/utilities/test_argparse_utils.py | 12 +++++++++- 6 files changed, 33 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index de8fec137c537..24fcc76c10ecf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,10 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). -## [UnReleased] - 2021-MM-DD +## [1.3.0] - 2021-MM-DD ### Added +- Added utils for NaN/Inf detection for gradients and parameters ([#6834](https://github.com/PyTorchLightning/pytorch-lightning/pull/6834/)) + - Added more explicit exception message when trying to execute `trainer.test()` or `trainer.validate()` with `fast_dev_run=True` ([#6667](https://github.com/PyTorchLightning/pytorch-lightning/pull/6667)) @@ -81,8 +83,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Added support for `precision=64`, enabling training with double precision ([#6595](https://github.com/PyTorchLightning/pytorch-lightning/pull/6595)) + - Added support for DDP communication hooks ([#6736](https://github.com/PyTorchLightning/pytorch-lightning/issues/6736)) + - Added `artifact_location` argument to `MLFlowLogger` which will be passed to the `MlflowClient.create_experiment` call ([#6677](https://github.com/PyTorchLightning/pytorch-lightning/pull/6677)) @@ -111,6 +115,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). ### Deprecated +- Deprecated `TrainerTrainingTricksMixin` in favor of a separate utilities module for NaN/Inf detection for gradients and parameters ([#6834](https://github.com/PyTorchLightning/pytorch-lightning/pull/6834/)) + + - `period` has been deprecated in favor of `every_n_val_epochs` in the `ModelCheckpoint` callback ([#6146](https://github.com/PyTorchLightning/pytorch-lightning/pull/6146)) @@ -221,19 +228,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Fixed `EarlyStopping` logic when `min_epochs` or `min_steps` requirement is not met ([#6705](https://github.com/PyTorchLightning/pytorch-lightning/pull/6705)) -## [1.2.8] - 2021-04-13 - - -### Changed - - -### Removed - - -### Fixed - - -- Fixed `EarlyStopping` logic when `min_epochs` or `min_steps` requirement is not met ([#6705](https://github.com/PyTorchLightning/pytorch-lightning/pull/6705)) +- Fixed `--gpus` default for parser returned by `Trainer.add_argparse_args` ([#6898](https://github.com/PyTorchLightning/pytorch-lightning/pull/6898)) ## [1.2.7] - 2021-04-06 diff --git a/pytorch_lightning/utilities/argparse.py b/pytorch_lightning/utilities/argparse.py index 2533dbc425948..071ec2fb29fa5 100644 --- a/pytorch_lightning/utilities/argparse.py +++ b/pytorch_lightning/utilities/argparse.py @@ -186,7 +186,6 @@ def add_argparse_args(cls, parent_parser: ArgumentParser) -> ArgumentParser: if arg == 'gpus' or arg == 'tpu_cores': use_type = _gpus_allowed_type - arg_default = _gpus_arg_default # hack for types in (int, float) if len(arg_types) == 2 and int in set(arg_types) and float in set(arg_types): @@ -238,13 +237,6 @@ def _gpus_allowed_type(x) -> Union[int, str]: return int(x) -def _gpus_arg_default(x) -> Union[int, str]: - if ',' in x: - return str(x) - else: - return int(x) - - def _int_or_float_type(x) -> Union[int, float]: if '.' in str(x): return float(x) diff --git a/pytorch_lightning/utilities/device_parser.py b/pytorch_lightning/utilities/device_parser.py index f20b978ebd8b6..f81a4ece1c6d0 100644 --- a/pytorch_lightning/utilities/device_parser.py +++ b/pytorch_lightning/utilities/device_parser.py @@ -59,11 +59,6 @@ def parse_gpu_ids(gpus: Optional[Union[int, str, List[int]]]) -> Optional[List[i If no GPUs are available but the value of gpus variable indicates request for GPUs then a MisconfigurationException is raised. """ - - # nothing was passed into the GPUs argument - if callable(gpus): - return None - # Check that gpus param is None, Int, String or List _check_data_type(gpus) @@ -97,10 +92,6 @@ def parse_tpu_cores(tpu_cores: Union[int, str, List]) -> Optional[Union[List[int Returns: a list of tpu_cores to be used or ``None`` if no TPU cores were requested """ - - if callable(tpu_cores): - return None - _check_data_type(tpu_cores) if isinstance(tpu_cores, str): diff --git a/tests/loggers/test_wandb.py b/tests/loggers/test_wandb.py index 0eefb9625ddc7..49c669a8f0aee 100644 --- a/tests/loggers/test_wandb.py +++ b/tests/loggers/test_wandb.py @@ -13,7 +13,6 @@ # limitations under the License. import os import pickle -import types from argparse import ArgumentParser from unittest import mock @@ -172,11 +171,10 @@ def wrapper_something(): params.wrapper_something_wo_name = lambda: lambda: '1' params.wrapper_something = wrapper_something - assert isinstance(params.gpus, types.FunctionType) params = WandbLogger._convert_params(params) params = WandbLogger._flatten_dict(params) params = WandbLogger._sanitize_callable_params(params) - assert params["gpus"] == '_gpus_arg_default' + assert params["gpus"] == "None" assert params["something"] == "something" assert params["wrapper_something"] == "wrapper_something" assert params["wrapper_something_wo_name"] == "" diff --git a/tests/trainer/test_trainer_cli.py b/tests/trainer/test_trainer_cli.py index b15d0899fc424..59a5331588cb7 100644 --- a/tests/trainer/test_trainer_cli.py +++ b/tests/trainer/test_trainer_cli.py @@ -23,6 +23,7 @@ import tests.helpers.utils as tutils from pytorch_lightning import Trainer from pytorch_lightning.utilities import argparse +from tests.helpers.runif import RunIf @mock.patch('argparse.ArgumentParser.parse_args') @@ -45,7 +46,7 @@ def test_default_args(mock_argparse, tmpdir): @pytest.mark.parametrize('cli_args', [['--accumulate_grad_batches=22'], ['--weights_save_path=./'], []]) -def test_add_argparse_args_redefined(cli_args): +def test_add_argparse_args_redefined(cli_args: list): """Redefines some default Trainer arguments via the cli and tests the Trainer initialization correctness. """ @@ -90,7 +91,7 @@ def test_get_init_arguments_and_types(): @pytest.mark.parametrize('cli_args', [['--callbacks=1', '--logger'], ['--foo', '--bar=1']]) -def test_add_argparse_args_redefined_error(cli_args, monkeypatch): +def test_add_argparse_args_redefined_error(cli_args: list, monkeypatch): """Asserts thar an error raised in case of passing not default cli arguments.""" class _UnkArgError(Exception): @@ -171,12 +172,13 @@ def test_argparse_args_parsing(cli_args, expected): assert Trainer.from_argparse_args(args) -@pytest.mark.parametrize(['cli_args', 'expected_gpu'], [ - pytest.param('--gpus 1', [0]), - pytest.param('--gpus 0,', [0]), +@pytest.mark.parametrize(['cli_args', 'expected_parsed', 'expected_device_ids'], [ + pytest.param('', None, None), + pytest.param('--gpus 1', 1, [0]), + pytest.param('--gpus 0,', '0,', [0]), ]) -@pytest.mark.skipif(not torch.cuda.is_available(), reason="test requires GPU machine") -def test_argparse_args_parsing_gpus(cli_args, expected_gpu): +@RunIf(min_gpus=1) +def test_argparse_args_parsing_gpus(cli_args, expected_parsed, expected_device_ids): """Test multi type argument with bool.""" cli_args = cli_args.split(' ') if cli_args else [] with mock.patch("argparse._sys.argv", ["any.py"] + cli_args): @@ -184,14 +186,12 @@ def test_argparse_args_parsing_gpus(cli_args, expected_gpu): parser = Trainer.add_argparse_args(parent_parser=parser) args = Trainer.parse_argparser(parser) + assert args.gpus == expected_parsed trainer = Trainer.from_argparse_args(args) - assert trainer.data_parallel_device_ids == expected_gpu + assert trainer.data_parallel_device_ids == expected_device_ids -@pytest.mark.skipif( - sys.version_info < (3, 7), - reason="signature inspection while mocking is not working in Python < 3.7 despite autospec" -) +@RunIf(min_python="3.7.0") @pytest.mark.parametrize(['cli_args', 'extra_args'], [ pytest.param({}, {}), pytest.param({'logger': False}, {}), diff --git a/tests/utilities/test_argparse_utils.py b/tests/utilities/test_argparse_utils.py index 63227abf831ec..cf34e0ef1de57 100644 --- a/tests/utilities/test_argparse_utils.py +++ b/tests/utilities/test_argparse_utils.py @@ -1,4 +1,4 @@ -from pytorch_lightning.utilities.argparse import parse_args_from_docstring +from pytorch_lightning.utilities.argparse import parse_args_from_docstring, _gpus_allowed_type, _int_or_float_type def test_parse_args_from_docstring_normal(): @@ -48,3 +48,13 @@ def test_parse_args_from_docstring_empty(): """ ) assert len(args_help.keys()) == 0 + + +def test_gpus_allowed_type(): + assert _gpus_allowed_type('1,2') == '1,2' + assert _gpus_allowed_type('1') == 1 + + +def test_int_or_float_type(): + assert isinstance(_int_or_float_type('0.0'), float) + assert isinstance(_int_or_float_type('0'), int)