From 79ffbe27064cad00a504e8ec315131f29d2a0668 Mon Sep 17 00:00:00 2001 From: Mauricio Villegas Date: Tue, 14 Jun 2022 06:51:31 +0200 Subject: [PATCH 1/6] Added unit tests that checks expected init args with the new jsonargparse parameter resolvers. --- requirements/extra.txt | 2 +- tests/tests_pytorch/utilities/test_cli.py | 82 ++++++++++++++++++++++- 2 files changed, 80 insertions(+), 4 deletions(-) diff --git a/requirements/extra.txt b/requirements/extra.txt index e7d54903c2932..057d32f9b7688 100644 --- a/requirements/extra.txt +++ b/requirements/extra.txt @@ -3,6 +3,6 @@ matplotlib>3.1, <3.5.3 torchtext>=0.10.*, <=0.12.0 omegaconf>=2.0.5, <=2.1.* hydra-core>=1.0.5, <=1.1.* -jsonargparse[signatures]>=4.9.0, <=4.9.0 +jsonargparse[signatures]>=4.10.0.dev1 gcsfs>=2021.5.0, <=2022.2.0 rich>=10.2.2, !=10.15.0.a, <13.0.0 diff --git a/tests/tests_pytorch/utilities/test_cli.py b/tests/tests_pytorch/utilities/test_cli.py index 4049e09af885b..aaa914e229621 100644 --- a/tests/tests_pytorch/utilities/test_cli.py +++ b/tests/tests_pytorch/utilities/test_cli.py @@ -34,11 +34,20 @@ from pytorch_lightning import __version__, Callback, LightningDataModule, LightningModule, seed_everything, Trainer from pytorch_lightning.callbacks import LearningRateMonitor, ModelCheckpoint from pytorch_lightning.demos.boring_classes import BoringDataModule, BoringModel -from pytorch_lightning.loggers import Logger, TensorBoardLogger +from pytorch_lightning.loggers import ( + Logger, + TensorBoardLogger, + _COMET_AVAILABLE, + _NEPTUNE_AVAILABLE, + _WANDB_AVAILABLE, +) from pytorch_lightning.plugins.environments import SLURMEnvironment +from pytorch_lightning.profiler import PyTorchProfiler +from pytorch_lightning.strategies import DDPStrategy from pytorch_lightning.trainer.states import TrainerFn from pytorch_lightning.utilities import _TPU_AVAILABLE from pytorch_lightning.utilities.cli import ( + _JSONARGPARSE_SIGNATURES_AVAILABLE, _populate_registries, CALLBACK_REGISTRY, DATAMODULE_REGISTRY, @@ -61,6 +70,9 @@ if _TORCHVISION_AVAILABLE: torchvision_version = version.parse(__import__("torchvision").__version__) +if _JSONARGPARSE_SIGNATURES_AVAILABLE: + from jsonargparse import lazy_instance + @contextmanager def mock_subclasses(baseclass, *subclasses): @@ -1423,8 +1435,6 @@ def configure_optimizers(self, optimizer, lr_scheduler=None): def test_cli_parameter_with_lazy_instance_default(): - from jsonargparse import lazy_instance - class TestModel(BoringModel): def __init__(self, activation: torch.nn.Module = lazy_instance(torch.nn.LeakyReLU, negative_slope=0.05)): super().__init__() @@ -1440,6 +1450,20 @@ def __init__(self, activation: torch.nn.Module = lazy_instance(torch.nn.LeakyReL assert cli.model.activation is not model.activation +def test_ddpstrategy_lazy_instance_with_find_unused_parameters(): + with mock.patch("sys.argv", ["any.py"]): + cli = LightningCLI( + BoringModel, + run=False, + trainer_defaults={ + 'strategy': lazy_instance(DDPStrategy, find_unused_parameters=False) + } + ) + + assert cli.config.trainer.strategy.init_args.find_unused_parameters is False + assert isinstance(cli.config_init.trainer.strategy, DDPStrategy) + + def test_cli_logger_shorthand(): with mock.patch("sys.argv", ["any.py"]): cli = LightningCLI(TestModel, run=False, trainer_defaults={"logger": False}) @@ -1454,6 +1478,38 @@ def test_cli_logger_shorthand(): assert cli.trainer.logger is None +def _test_logger_init_args(logger_name, logger_init): + cli_args = [f"--trainer.logger={logger_name}"] + cli_args += [f"--trainer.logger.{k}={v}" for k, v in logger_init.items()] + cli_args.append("--print_config") + + out = StringIO() + with mock.patch("sys.argv", ["any.py"] + cli_args), redirect_stdout(out), pytest.raises(SystemExit): + LightningCLI(TestModel, run=False) + + init_args = yaml.safe_load(out.getvalue())["trainer"]["logger"]["init_args"] + assert {k: init_args[k] for k in logger_init} == logger_init + + +@pytest.mark.skipif(not _COMET_AVAILABLE, reason="comet-ml is required") +def test_comet_logger_init_args(): + _test_logger_init_args("CometLogger", {"save_dir": "comet", "workspace": "comet"}) + + +@pytest.mark.skipif(not _NEPTUNE_AVAILABLE, reason="neptune-client is required") +def test_neptune_logger_init_args(): + _test_logger_init_args("NeptuneLogger", {"name": "neptune", "__unresolved__": {"description": "neptune"}}) + + +def test_tensorboard_logger_init_args(): + _test_logger_init_args("TensorBoardLogger", {"save_dir": "tb", "name": "tb"}) + + +@pytest.mark.skipif(not _WANDB_AVAILABLE, reason="wandb is required") +def test_wandb_logger_init_args(): + _test_logger_init_args("WandbLogger", {"save_dir": "wandb", "notes": "wandb"}) + + def test_cli_auto_seeding(): with mock.patch("sys.argv", ["any.py"]): cli = LightningCLI(TestModel, run=False, seed_everything_default=False) @@ -1513,3 +1569,23 @@ def __init__(self, a_func: Callable = torch.softmax): LightningCLI(TestModel, run=False) assert "a_func: torch.softmax" in out.getvalue() + + +def test_pytorch_profiler_init_args(): + profiler_init = { + "dirpath": "profiler", + "row_limit": 10, + "group_by_input_shapes": True, + "__unresolved__": { + "profile_memory": True, + "record_shapes": True, + }, + } + cli_args = ["--trainer.profiler=PyTorchProfiler"] + cli_args += [f"--trainer.profiler.{k}={v}" for k, v in profiler_init.items()] + + with mock.patch("sys.argv", ["any.py"] + cli_args): + cli = LightningCLI(TestModel, run=False) + + assert isinstance(cli.config_init.trainer.profiler, PyTorchProfiler) + assert {k: cli.config.trainer.profiler.init_args[k] for k in profiler_init} == profiler_init From c42e972f42e1163e556b2e33b30cfd19d18a6ef0 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 14 Jun 2022 05:20:35 +0000 Subject: [PATCH 2/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/tests_pytorch/utilities/test_cli.py | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/tests/tests_pytorch/utilities/test_cli.py b/tests/tests_pytorch/utilities/test_cli.py index aaa914e229621..36a6e919dee4d 100644 --- a/tests/tests_pytorch/utilities/test_cli.py +++ b/tests/tests_pytorch/utilities/test_cli.py @@ -33,14 +33,7 @@ from pytorch_lightning import __version__, Callback, LightningDataModule, LightningModule, seed_everything, Trainer from pytorch_lightning.callbacks import LearningRateMonitor, ModelCheckpoint -from pytorch_lightning.demos.boring_classes import BoringDataModule, BoringModel -from pytorch_lightning.loggers import ( - Logger, - TensorBoardLogger, - _COMET_AVAILABLE, - _NEPTUNE_AVAILABLE, - _WANDB_AVAILABLE, -) +from pytorch_lightning.loggers import _COMET_AVAILABLE, _NEPTUNE_AVAILABLE, _WANDB_AVAILABLE, Logger, TensorBoardLogger from pytorch_lightning.plugins.environments import SLURMEnvironment from pytorch_lightning.profiler import PyTorchProfiler from pytorch_lightning.strategies import DDPStrategy @@ -1455,9 +1448,7 @@ def test_ddpstrategy_lazy_instance_with_find_unused_parameters(): cli = LightningCLI( BoringModel, run=False, - trainer_defaults={ - 'strategy': lazy_instance(DDPStrategy, find_unused_parameters=False) - } + trainer_defaults={"strategy": lazy_instance(DDPStrategy, find_unused_parameters=False)}, ) assert cli.config.trainer.strategy.init_args.find_unused_parameters is False From 9ac5250e8c425763c998b6bdfd922908e1331bce Mon Sep 17 00:00:00 2001 From: Mauricio Villegas Date: Tue, 14 Jun 2022 07:23:01 +0200 Subject: [PATCH 3/6] Added entry in changelog. --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f4623e03599c8..568f58bd4b779 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -214,6 +214,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Fixed issue where the CLI fails with certain torch objects ([#13153](https://github.com/PyTorchLightning/pytorch-lightning/pull/13153)) +- Fixed ``LightningCLI`` signature parameter resolving for some lightning classes ([#13283](https://github.com/PyTorchLightning/pytorch-lightning/pull/13283)) + + - From b1c39845955462e80611b6c8be19b86549551208 Mon Sep 17 00:00:00 2001 From: Mauricio Villegas Date: Thu, 16 Jun 2022 22:30:55 +0200 Subject: [PATCH 4/6] Improvements based on review --- requirements/extra.txt | 2 +- tests/tests_pytorch/utilities/test_cli.py | 41 ++++++++++++++--------- 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/requirements/extra.txt b/requirements/extra.txt index 057d32f9b7688..6f7100b70dfa9 100644 --- a/requirements/extra.txt +++ b/requirements/extra.txt @@ -3,6 +3,6 @@ matplotlib>3.1, <3.5.3 torchtext>=0.10.*, <=0.12.0 omegaconf>=2.0.5, <=2.1.* hydra-core>=1.0.5, <=1.1.* -jsonargparse[signatures]>=4.10.0.dev1 +jsonargparse[signatures]>=4.10.0.dev2 gcsfs>=2021.5.0, <=2022.2.0 rich>=10.2.2, !=10.15.0.a, <13.0.0 diff --git a/tests/tests_pytorch/utilities/test_cli.py b/tests/tests_pytorch/utilities/test_cli.py index 36a6e919dee4d..3901503483d61 100644 --- a/tests/tests_pytorch/utilities/test_cli.py +++ b/tests/tests_pytorch/utilities/test_cli.py @@ -33,6 +33,7 @@ from pytorch_lightning import __version__, Callback, LightningDataModule, LightningModule, seed_everything, Trainer from pytorch_lightning.callbacks import LearningRateMonitor, ModelCheckpoint +from pytorch_lightning.demos.boring_classes import BoringDataModule, BoringModel from pytorch_lightning.loggers import _COMET_AVAILABLE, _NEPTUNE_AVAILABLE, _WANDB_AVAILABLE, Logger, TensorBoardLogger from pytorch_lightning.plugins.environments import SLURMEnvironment from pytorch_lightning.profiler import PyTorchProfiler @@ -1443,16 +1444,19 @@ def __init__(self, activation: torch.nn.Module = lazy_instance(torch.nn.LeakyReL assert cli.model.activation is not model.activation -def test_ddpstrategy_lazy_instance_with_find_unused_parameters(): - with mock.patch("sys.argv", ["any.py"]): +def test_ddpstrategy_instantiation_and_find_unused_parameters(): + strategy_default = lazy_instance(DDPStrategy, find_unused_parameters=True) + with mock.patch("sys.argv", ["any.py", "--trainer.strategy.process_group_backend=group"]): cli = LightningCLI( BoringModel, run=False, - trainer_defaults={"strategy": lazy_instance(DDPStrategy, find_unused_parameters=False)}, + trainer_defaults={"strategy": strategy_default}, ) - assert cli.config.trainer.strategy.init_args.find_unused_parameters is False + assert cli.config.trainer.strategy.init_args.find_unused_parameters is True assert isinstance(cli.config_init.trainer.strategy, DDPStrategy) + assert cli.config_init.trainer.strategy.process_group_backend == "group" + assert strategy_default is not cli.config_init.trainer.strategy def test_cli_logger_shorthand(): @@ -1469,17 +1473,20 @@ def test_cli_logger_shorthand(): assert cli.trainer.logger is None -def _test_logger_init_args(logger_name, logger_init): +def _test_logger_init_args(logger_name, init, unresolved = {}): cli_args = [f"--trainer.logger={logger_name}"] - cli_args += [f"--trainer.logger.{k}={v}" for k, v in logger_init.items()] + cli_args += [f"--trainer.logger.{k}={v}" for k, v in init.items()] + cli_args += [f"--trainer.logger.dict_kwargs.{k}={v}" for k, v in unresolved.items()] cli_args.append("--print_config") out = StringIO() with mock.patch("sys.argv", ["any.py"] + cli_args), redirect_stdout(out), pytest.raises(SystemExit): LightningCLI(TestModel, run=False) - init_args = yaml.safe_load(out.getvalue())["trainer"]["logger"]["init_args"] - assert {k: init_args[k] for k in logger_init} == logger_init + data = yaml.safe_load(out.getvalue())["trainer"]["logger"] + assert {k: data['init_args'][k] for k in init} == init + if unresolved: + assert data['dict_kwargs'] == unresolved @pytest.mark.skipif(not _COMET_AVAILABLE, reason="comet-ml is required") @@ -1489,7 +1496,7 @@ def test_comet_logger_init_args(): @pytest.mark.skipif(not _NEPTUNE_AVAILABLE, reason="neptune-client is required") def test_neptune_logger_init_args(): - _test_logger_init_args("NeptuneLogger", {"name": "neptune", "__unresolved__": {"description": "neptune"}}) + _test_logger_init_args("NeptuneLogger", {"name": "neptune"}, {"description": "neptune"}) def test_tensorboard_logger_init_args(): @@ -1563,20 +1570,22 @@ def __init__(self, a_func: Callable = torch.softmax): def test_pytorch_profiler_init_args(): - profiler_init = { + init = { "dirpath": "profiler", "row_limit": 10, "group_by_input_shapes": True, - "__unresolved__": { - "profile_memory": True, - "record_shapes": True, - }, + } + unresolved = { + "profile_memory": True, + "record_shapes": True, } cli_args = ["--trainer.profiler=PyTorchProfiler"] - cli_args += [f"--trainer.profiler.{k}={v}" for k, v in profiler_init.items()] + cli_args += [f"--trainer.profiler.{k}={v}" for k, v in init.items()] + cli_args += [f"--trainer.profiler.dict_kwargs.{k}={v}" for k, v in unresolved.items()] with mock.patch("sys.argv", ["any.py"] + cli_args): cli = LightningCLI(TestModel, run=False) assert isinstance(cli.config_init.trainer.profiler, PyTorchProfiler) - assert {k: cli.config.trainer.profiler.init_args[k] for k in profiler_init} == profiler_init + assert {k: cli.config.trainer.profiler.init_args[k] for k in init} == init + assert cli.config.trainer.profiler.dict_kwargs == unresolved From 34fe10a8da95531f82489f96fd209a55e3095c84 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 16 Jun 2022 20:47:37 +0000 Subject: [PATCH 5/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- README.md | 3 +-- tests/tests_pytorch/utilities/test_cli.py | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 39b42b0f2f9aa..95b28c70b120b 100644 --- a/README.md +++ b/README.md @@ -36,14 +36,13 @@ ______________________________________________________________________ ###### \*Codecov is > 90%+ but build delays may show less ----- +______________________________________________________________________ ## PyTorch Lightning is just organized PyTorch Lightning disentangles PyTorch code to decouple the science from the engineering. ![PT to PL](docs/source-pytorch/_static/images/general/pl_quick_start_full_compressed.gif) - ## Build AI products with Lightning Apps Once you're done building models, publish a paper demo or build a full production end-to-end ML system with Lightning Apps. Lightning Apps remove the cloud infrastructure boilerplate so you can focus on solving the research or business problems. Lightning Apps can run on the Lightning Cloud, your own cluster or a private cloud. diff --git a/tests/tests_pytorch/utilities/test_cli.py b/tests/tests_pytorch/utilities/test_cli.py index 3901503483d61..9da53ed47d83a 100644 --- a/tests/tests_pytorch/utilities/test_cli.py +++ b/tests/tests_pytorch/utilities/test_cli.py @@ -1473,7 +1473,7 @@ def test_cli_logger_shorthand(): assert cli.trainer.logger is None -def _test_logger_init_args(logger_name, init, unresolved = {}): +def _test_logger_init_args(logger_name, init, unresolved={}): cli_args = [f"--trainer.logger={logger_name}"] cli_args += [f"--trainer.logger.{k}={v}" for k, v in init.items()] cli_args += [f"--trainer.logger.dict_kwargs.{k}={v}" for k, v in unresolved.items()] @@ -1484,9 +1484,9 @@ def _test_logger_init_args(logger_name, init, unresolved = {}): LightningCLI(TestModel, run=False) data = yaml.safe_load(out.getvalue())["trainer"]["logger"] - assert {k: data['init_args'][k] for k in init} == init + assert {k: data["init_args"][k] for k in init} == init if unresolved: - assert data['dict_kwargs'] == unresolved + assert data["dict_kwargs"] == unresolved @pytest.mark.skipif(not _COMET_AVAILABLE, reason="comet-ml is required") From 75888d4407c8e874ca699bf27eb098248cab5647 Mon Sep 17 00:00:00 2001 From: Mauricio Villegas Date: Tue, 21 Jun 2022 07:47:24 +0200 Subject: [PATCH 6/6] Update jsonargparse requirement to 4.10.0. --- requirements/extra.txt | 2 +- src/pytorch_lightning/utilities/cli.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements/extra.txt b/requirements/extra.txt index 6f7100b70dfa9..9da03791d6429 100644 --- a/requirements/extra.txt +++ b/requirements/extra.txt @@ -3,6 +3,6 @@ matplotlib>3.1, <3.5.3 torchtext>=0.10.*, <=0.12.0 omegaconf>=2.0.5, <=2.1.* hydra-core>=1.0.5, <=1.1.* -jsonargparse[signatures]>=4.10.0.dev2 +jsonargparse[signatures]>=4.10.0, <=4.10.0 gcsfs>=2021.5.0, <=2022.2.0 rich>=10.2.2, !=10.15.0.a, <13.0.0 diff --git a/src/pytorch_lightning/utilities/cli.py b/src/pytorch_lightning/utilities/cli.py index 6a2b32432ec7e..e6b52e40d4544 100644 --- a/src/pytorch_lightning/utilities/cli.py +++ b/src/pytorch_lightning/utilities/cli.py @@ -31,7 +31,7 @@ from pytorch_lightning.utilities.model_helpers import is_overridden from pytorch_lightning.utilities.rank_zero import _warn, rank_zero_deprecation, rank_zero_warn -_JSONARGPARSE_SIGNATURES_AVAILABLE = _RequirementAvailable("jsonargparse[signatures]>=4.9.0") +_JSONARGPARSE_SIGNATURES_AVAILABLE = _RequirementAvailable("jsonargparse[signatures]>=4.10.0") if _JSONARGPARSE_SIGNATURES_AVAILABLE: import docstring_parser