From 03435993cd87c732fcf9220b0fc5b82221928c03 Mon Sep 17 00:00:00 2001 From: Kaushik B Date: Wed, 10 Nov 2021 12:42:40 +0530 Subject: [PATCH 1/7] Remove deprecated mode argument from ModelSummary --- pytorch_lightning/core/lightning.py | 9 +--- pytorch_lightning/utilities/model_summary.py | 54 ++------------------ tests/deprecated_api/test_remove_1-6.py | 10 ---- 3 files changed, 7 insertions(+), 66 deletions(-) diff --git a/pytorch_lightning/core/lightning.py b/pytorch_lightning/core/lightning.py index 7867211badb35..8c68da65a1439 100644 --- a/pytorch_lightning/core/lightning.py +++ b/pytorch_lightning/core/lightning.py @@ -1689,7 +1689,7 @@ def tbptt_split_batch(self, batch, split_size): return splits - def summarize(self, mode: Optional[str] = "top", max_depth: Optional[int] = None) -> Optional[ModelSummary]: + def summarize(self, max_depth: Optional[int] = None) -> Optional[ModelSummary]: """Summarize this LightningModule. .. deprecated:: v1.5 @@ -1697,11 +1697,6 @@ def summarize(self, mode: Optional[str] = "top", max_depth: Optional[int] = None and will be removed in v1.7. Args: - mode: Can be either ``'top'`` (summarize only direct submodules) or ``'full'`` (summarize all layers). - - .. deprecated:: v1.4 - This parameter was deprecated in v1.4 in favor of `max_depth` and will be removed in v1.6. - max_depth: The maximum depth of layer nesting that the summary will include. A value of 0 turns the layer summary off. Default: 1. @@ -1714,7 +1709,7 @@ def summarize(self, mode: Optional[str] = "top", max_depth: Optional[int] = None stacklevel=6, ) - return summarize(self, mode, max_depth) + return summarize(self, max_depth) def freeze(self) -> None: r""" diff --git a/pytorch_lightning/utilities/model_summary.py b/pytorch_lightning/utilities/model_summary.py index 9c2690202df90..c55cae4af56e4 100644 --- a/pytorch_lightning/utilities/model_summary.py +++ b/pytorch_lightning/utilities/model_summary.py @@ -23,8 +23,7 @@ from torch.utils.hooks import RemovableHandle import pytorch_lightning as pl -from pytorch_lightning.utilities import AMPType, DeviceType, ModelSummaryMode, rank_zero_deprecation -from pytorch_lightning.utilities.exceptions import MisconfigurationException +from pytorch_lightning.utilities import AMPType, DeviceType from pytorch_lightning.utilities.imports import _TORCH_GREATER_EQUAL_1_8 from pytorch_lightning.utilities.warnings import WarningCache @@ -130,13 +129,6 @@ class ModelSummary: Args: model: The model to summarize (also referred to as the root module). - mode: Can be one of - - - `top` (default): only the top-level modules will be recorded (the children of the root module) - - `full`: summarizes all layers and their submodules in the root module - - .. deprecated:: v1.4 - This parameter was deprecated in v1.4 in favor of `max_depth` and will be removed in v1.6. max_depth: Maximum depth of modules to show. Use -1 to show all modules or 0 to show no summary. Defaults to 1. @@ -186,22 +178,9 @@ class ModelSummary: 0.530 Total estimated model params size (MB) """ - def __init__(self, model: "pl.LightningModule", mode: Optional[str] = None, max_depth: Optional[int] = 1) -> None: + def __init__(self, model: "pl.LightningModule", max_depth: Optional[int] = 1) -> None: self._model = model - # temporary mapping from mode to max_depth - if max_depth is None or mode is not None: - if mode in ModelSummaryMode.supported_types(): - max_depth = ModelSummaryMode.get_max_depth(mode) - rank_zero_deprecation( - "Argument `mode` in `ModelSummary` is deprecated in v1.4" - f" and will be removed in v1.6. Use `max_depth={max_depth}` to replicate `mode={mode}` behaviour." - ) - else: - raise MisconfigurationException( - f"`mode` can be {', '.join(ModelSummaryMode.supported_types())}, got {mode}." - ) - if not isinstance(max_depth, int) or max_depth < -1: raise ValueError(f"`max_depth` can be -1, 0 or > 0, got {max_depth}.") @@ -436,17 +415,11 @@ def _is_lazy_weight_tensor(p: Tensor) -> bool: return False -def summarize( - lightning_module: "pl.LightningModule", mode: Optional[str] = None, max_depth: Optional[int] = None -) -> ModelSummary: +def summarize(lightning_module: "pl.LightningModule", max_depth: Optional[int] = None) -> ModelSummary: """Summarize the LightningModule specified by `lightning_module`. Args: lightning_module: `LightningModule` to summarize. - mode: Can be either ``'top'`` (summarize only direct submodules) or ``'full'`` (summarize all layers). - - .. deprecated:: v1.4 - This parameter was deprecated in v1.4 in favor of `max_depth` and will be removed in v1.6. max_depth: The maximum depth of layer nesting that the summary will include. A value of 0 turns the layer summary off. Default: 1. @@ -454,22 +427,5 @@ def summarize( Return: The model summary object """ - - # temporary mapping from mode to max_depth - if max_depth is None: - if mode is None: - model_summary = ModelSummary(lightning_module, max_depth=1) - elif mode in ModelSummaryMode.supported_types(): - max_depth = ModelSummaryMode.get_max_depth(mode) - rank_zero_deprecation( - "Argument `mode` in `LightningModule.summarize` is deprecated in v1.4" - f" and will be removed in v1.6. Use `max_depth={max_depth}` to replicate `mode={mode}` behavior." - ) - model_summary = ModelSummary(lightning_module, max_depth=max_depth) - else: - raise MisconfigurationException( - f"`mode` can be None, {', '.join(ModelSummaryMode.supported_types())}, got {mode}" - ) - else: - model_summary = ModelSummary(lightning_module, max_depth=max_depth) - return model_summary + max_depth = max_depth or 1 + return ModelSummary(lightning_module, max_depth=max_depth) diff --git a/tests/deprecated_api/test_remove_1-6.py b/tests/deprecated_api/test_remove_1-6.py index a64d28ebefecc..8660a9b3472b6 100644 --- a/tests/deprecated_api/test_remove_1-6.py +++ b/tests/deprecated_api/test_remove_1-6.py @@ -19,7 +19,6 @@ from pytorch_lightning import Trainer from pytorch_lightning.utilities.distributed import rank_zero_deprecation, rank_zero_warn from pytorch_lightning.utilities.model_helpers import is_overridden -from pytorch_lightning.utilities.model_summary import ModelSummary from tests.helpers import BoringModel @@ -85,15 +84,6 @@ def test_v1_6_0_rank_zero_warnings_moved(): rank_zero_deprecation("test") -def test_v1_6_0_deprecated_model_summary_mode(tmpdir): - model = BoringModel() - with pytest.deprecated_call(match="Argument `mode` in `ModelSummary` is deprecated in v1.4"): - ModelSummary(model, mode="top") - - with pytest.deprecated_call(match="Argument `mode` in `LightningModule.summarize` is deprecated in v1.4"): - model.summarize(mode="top") - - def test_v1_6_0_deprecated_disable_validation(): trainer = Trainer() with pytest.deprecated_call(match="disable_validation` is deprecated in v1.4"): From 49b805e102bd6f94eca69505980e0ac0714c9570 Mon Sep 17 00:00:00 2001 From: Kaushik B Date: Wed, 10 Nov 2021 12:56:40 +0530 Subject: [PATCH 2/7] Update ModelSummary tests --- tests/utilities/test_model_summary.py | 69 +++++++++++---------------- 1 file changed, 27 insertions(+), 42 deletions(-) diff --git a/tests/utilities/test_model_summary.py b/tests/utilities/test_model_summary.py index 1f44e0a74f68d..e6ba650f55891 100644 --- a/tests/utilities/test_model_summary.py +++ b/tests/utilities/test_model_summary.py @@ -141,31 +141,29 @@ def forward(self, inp): def test_invalid_weights_summmary(): """Test that invalid value for weights_summary raises an error.""" - with pytest.raises(MisconfigurationException, match="`mode` can be None, .* got temp"): - summarize(UnorderedModel, mode="temp") with pytest.raises(MisconfigurationException, match="`weights_summary` can be None, .* got temp"): Trainer(weights_summary="temp") -@pytest.mark.parametrize("mode", ["full", "top"]) -def test_empty_model_summary_shapes(mode: str): +@pytest.mark.parametrize("max_depth", [-1, 1]) +def test_empty_model_summary_shapes(max_depth: int): """Test that the summary works for models that have no submodules.""" model = EmptyModule() - summary = summarize(model, mode=mode) + summary = summarize(model, max_depth) assert summary.in_sizes == [] assert summary.out_sizes == [] assert summary.param_nums == [] @RunIf(min_gpus=1) -@pytest.mark.parametrize("mode", ["full", "top"]) +@pytest.mark.parametrize("max_depth", [-1, 1]) @pytest.mark.parametrize("device", [torch.device("cpu"), torch.device("cuda", 0)]) -def test_linear_model_summary_shapes(device, mode): +def test_linear_model_summary_shapes(device, max_depth): """Test that the model summary correctly computes the input- and output shapes.""" model = UnorderedModel().to(device) model.train() - summary = summarize(model, mode=mode) + summary = summarize(model, max_depth) assert summary.in_sizes == [[2, 10], [2, 7], [2, 3], [2, 7], UNKNOWN_SIZE] # layer 2 # combine # layer 1 # relu assert summary.out_sizes == [[2, 2], [2, 9], [2, 5], [2, 7], UNKNOWN_SIZE] # layer 2 # combine # layer 1 # relu assert model.training @@ -191,8 +189,8 @@ def test_hooks_removed_after_summarize(max_depth): assert handle.id not in handle.hooks_dict_ref() -@pytest.mark.parametrize("mode", ["full", "top"]) -def test_rnn_summary_shapes(mode): +@pytest.mark.parametrize("max_depth", [-1, 1]) +def test_rnn_summary_shapes(max_depth): """Test that the model summary works for RNNs.""" model = ParityModuleRNN() @@ -204,16 +202,16 @@ def test_rnn_summary_shapes(mode): model.example_input_array = torch.zeros(b, t, 10) - summary = summarize(model, mode=mode) + summary = summarize(model, max_depth) assert summary.in_sizes == [[b, t, i], [b, t, h]] # rnn # linear assert summary.out_sizes == [[[b, t, h], [[1, b, h], [1, b, h]]], [b, t, o]] # rnn # linear -@pytest.mark.parametrize("mode", ["full", "top"]) -def test_summary_parameter_count(mode): +@pytest.mark.parametrize("max_depth", [-1, 1]) +def test_summary_parameter_count(max_depth): """Test that the summary counts the number of parameters in every submodule.""" model = UnorderedModel() - summary = summarize(model, mode=mode) + summary = summarize(model, max_depth) assert summary.param_nums == [ model.layer2.weight.numel() + model.layer2.bias.numel(), model.combine.weight.numel() + model.combine.bias.numel(), @@ -223,24 +221,24 @@ def test_summary_parameter_count(mode): ] -@pytest.mark.parametrize("mode", ["full", "top"]) -def test_summary_layer_types(mode): +@pytest.mark.parametrize("max_depth", [-1, 1]) +def test_summary_layer_types(max_depth): """Test that the summary displays the layer names correctly.""" model = UnorderedModel() - summary = summarize(model, mode=mode) + summary = summarize(model, max_depth) assert summary.layer_types == ["Linear", "Linear", "Linear", "ReLU", "Conv2d"] -@pytest.mark.parametrize("mode", ["full", "top"]) -def test_summary_with_scripted_modules(mode): +@pytest.mark.parametrize("max_depth", [-1, 1]) +def test_summary_with_scripted_modules(max_depth): model = PartialScriptModel() - summary = summarize(model, mode=mode) + summary = summarize(model, max_depth) assert summary.layer_types == ["RecursiveScriptModule", "Linear"] assert summary.in_sizes == [UNKNOWN_SIZE, [2, 3]] assert summary.out_sizes == [UNKNOWN_SIZE, [2, 2]] -@pytest.mark.parametrize("mode", ["full", "top"]) +@pytest.mark.parametrize("max_depth", [-1, 1]) @pytest.mark.parametrize( ["example_input", "expected_size"], [ @@ -253,7 +251,7 @@ def test_summary_with_scripted_modules(mode): ((torch.zeros(2, 3), torch.zeros(4, 5)), [[2, 3], [4, 5]]), ], ) -def test_example_input_array_types(example_input, expected_size, mode): +def test_example_input_array_types(example_input, expected_size, max_depth): """Test the types of example inputs supported for display in the summary.""" class DummyModule(nn.Module): @@ -271,23 +269,23 @@ def forward(self, *args, **kwargs): model = DummyLightningModule() model.example_input_array = example_input - summary = summarize(model, mode=mode) + summary = summarize(model, max_depth) assert summary.in_sizes == [expected_size] -@pytest.mark.parametrize("mode", ["full", "top"]) -def test_model_size(mode): +@pytest.mark.parametrize("max_depth", [-1, 1]) +def test_model_size(max_depth): """Test model size is calculated correctly.""" model = PreCalculatedModel() - summary = summarize(model, mode=mode) + summary = summarize(model, max_depth) assert model.pre_calculated_model_size == summary.model_size -@pytest.mark.parametrize("mode", ["full", "top"]) -def test_empty_model_size(mode): +@pytest.mark.parametrize("max_depth", [-1, 1]) +def test_empty_model_size(max_depth): """Test empty model size is zero.""" model = EmptyModule() - summary = summarize(model, mode=mode) + summary = summarize(model, max_depth) assert 0.0 == summary.model_size @@ -324,19 +322,6 @@ def test_lazy_model_summary(): assert summary.trainable_parameters == 7 -def test_max_depth_equals_mode_interface(): - """Test summarize(model, full/top) interface mapping matches max_depth.""" - model = DeepNestedModel() - - summary_top = summarize(model, mode="top") - summary_0 = summarize(model, max_depth=1) - assert str(summary_top) == str(summary_0) - - summary_full = summarize(model, mode="full") - summary_minus1 = summarize(model, max_depth=-1) - assert str(summary_full) == str(summary_minus1) - - @pytest.mark.parametrize("max_depth", [-1, 0, 1, 3, 999]) def test_max_depth_param(max_depth): """Test that only the modules up to the desired depth are shown.""" From 3307be52c9893666fcce20a0ef624b9c67fb5b6d Mon Sep 17 00:00:00 2001 From: Kaushik B Date: Wed, 10 Nov 2021 13:00:21 +0530 Subject: [PATCH 3/7] Update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a8526a360b49d..7b9189b8c3793 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -108,6 +108,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Removed deprecated passthrough methods and properties from `Accelerator` base class ([#10403](https://github.com/PyTorchLightning/pytorch-lightning/pull/10403)) +- Removed deprecated `mode` argument from `ModelSummary` class ([#10449](https://github.com/PyTorchLightning/pytorch-lightning/pull/10449)) + + ### Fixed - Fixed an issue where class or init-only variables of dataclasses were passed to the dataclass constructor in `utilities.apply_to_collection` ([#9702](https://github.com/PyTorchLightning/pytorch-lightning/issues/9702)) From 926d20f8fc80a2a2dfabf21d54f2101ccd2e94af Mon Sep 17 00:00:00 2001 From: Kaushik B Date: Wed, 10 Nov 2021 13:20:54 +0530 Subject: [PATCH 4/7] Update max_depth default to 1 --- pytorch_lightning/core/lightning.py | 2 +- pytorch_lightning/utilities/model_summary.py | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/pytorch_lightning/core/lightning.py b/pytorch_lightning/core/lightning.py index 8c68da65a1439..169def5cc6119 100644 --- a/pytorch_lightning/core/lightning.py +++ b/pytorch_lightning/core/lightning.py @@ -1689,7 +1689,7 @@ def tbptt_split_batch(self, batch, split_size): return splits - def summarize(self, max_depth: Optional[int] = None) -> Optional[ModelSummary]: + def summarize(self, max_depth: int = 1) -> ModelSummary: """Summarize this LightningModule. .. deprecated:: v1.5 diff --git a/pytorch_lightning/utilities/model_summary.py b/pytorch_lightning/utilities/model_summary.py index c55cae4af56e4..bab6da5368b65 100644 --- a/pytorch_lightning/utilities/model_summary.py +++ b/pytorch_lightning/utilities/model_summary.py @@ -178,7 +178,7 @@ class ModelSummary: 0.530 Total estimated model params size (MB) """ - def __init__(self, model: "pl.LightningModule", max_depth: Optional[int] = 1) -> None: + def __init__(self, model: "pl.LightningModule", max_depth: int = 1) -> None: self._model = model if not isinstance(max_depth, int) or max_depth < -1: @@ -415,7 +415,7 @@ def _is_lazy_weight_tensor(p: Tensor) -> bool: return False -def summarize(lightning_module: "pl.LightningModule", max_depth: Optional[int] = None) -> ModelSummary: +def summarize(lightning_module: "pl.LightningModule", max_depth: int = 1) -> ModelSummary: """Summarize the LightningModule specified by `lightning_module`. Args: @@ -427,5 +427,4 @@ def summarize(lightning_module: "pl.LightningModule", max_depth: Optional[int] = Return: The model summary object """ - max_depth = max_depth or 1 return ModelSummary(lightning_module, max_depth=max_depth) From 4efa46961406c779b5d822f4d30517af9bf0ae4d Mon Sep 17 00:00:00 2001 From: Kaushik B <45285388+kaushikb11@users.noreply.github.com> Date: Fri, 12 Nov 2021 14:20:33 +0530 Subject: [PATCH 5/7] Update tests/deprecated_api/test_remove_1-6.py --- tests/deprecated_api/test_remove_1-6.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/deprecated_api/test_remove_1-6.py b/tests/deprecated_api/test_remove_1-6.py index 0523976b36db7..d3f6751e95f3d 100644 --- a/tests/deprecated_api/test_remove_1-6.py +++ b/tests/deprecated_api/test_remove_1-6.py @@ -76,11 +76,6 @@ def test_v1_6_0_train_loop(tmpdir): _ = trainer.train_loop -def test_v1_6_0_rank_zero_warnings_moved(): - with pytest.deprecated_call(match="in v1.3.7 and will be removed in v1.6"): - rank_zero_warn("test") - with pytest.deprecated_call(match="in v1.3.7 and will be removed in v1.6"): - rank_zero_deprecation("test") def test_v1_6_0_deprecated_disable_validation(): From 73c1999c8c85a1123455776f0c27afa124a909cc Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 12 Nov 2021 08:51:51 +0000 Subject: [PATCH 6/7] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/deprecated_api/test_remove_1-6.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/deprecated_api/test_remove_1-6.py b/tests/deprecated_api/test_remove_1-6.py index d3f6751e95f3d..126fe7da45f58 100644 --- a/tests/deprecated_api/test_remove_1-6.py +++ b/tests/deprecated_api/test_remove_1-6.py @@ -76,8 +76,6 @@ def test_v1_6_0_train_loop(tmpdir): _ = trainer.train_loop - - def test_v1_6_0_deprecated_disable_validation(): trainer = Trainer() with pytest.deprecated_call(match="disable_validation` is deprecated in v1.4"): From 81adec0a2f12ec0d32b454352c4a82e8d46fc534 Mon Sep 17 00:00:00 2001 From: Kaushik B <45285388+kaushikb11@users.noreply.github.com> Date: Fri, 12 Nov 2021 15:49:48 +0530 Subject: [PATCH 7/7] Update tests/utilities/test_model_summary.py --- tests/utilities/test_model_summary.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/utilities/test_model_summary.py b/tests/utilities/test_model_summary.py index 056d81a49270a..bc465e78fee38 100644 --- a/tests/utilities/test_model_summary.py +++ b/tests/utilities/test_model_summary.py @@ -141,6 +141,7 @@ def forward(self, inp): def test_invalid_weights_summmary(): """Test that invalid value for weights_summary raises an error.""" + model = LightningModule() with pytest.raises( MisconfigurationException, match="`weights_summary` can be None, .* got temp"