From e6069f1cf53fc403e62423df10d92a7fb7adfbcb Mon Sep 17 00:00:00 2001 From: AndresAlgaba Date: Fri, 23 Sep 2022 12:24:26 +0200 Subject: [PATCH 1/5] remove on_init_start_end --- docs/source-pytorch/extensions/callbacks.rst | 12 -------- src/lightning_app/utilities/introspection.py | 2 -- src/pytorch_lightning/CHANGELOG.md | 4 +++ src/pytorch_lightning/callbacks/callback.py | 16 ---------- .../callbacks/lambda_function.py | 2 -- src/pytorch_lightning/core/module.py | 4 --- .../trainer/configuration_validator.py | 7 ----- .../logger_connector/fx_validator.py | 2 -- src/pytorch_lightning/trainer/trainer.py | 15 ---------- .../tests_pytorch/callbacks/test_callbacks.py | 7 +---- .../callbacks/test_lambda_function.py | 20 +++---------- .../deprecated_api/test_remove_1-8.py | 28 ----------------- tests/tests_pytorch/models/test_hooks.py | 30 ------------------- .../trainer/logging_/test_logger_connector.py | 8 ----- 14 files changed, 9 insertions(+), 148 deletions(-) diff --git a/docs/source-pytorch/extensions/callbacks.rst b/docs/source-pytorch/extensions/callbacks.rst index 72f02fadb6af6..f6d65a143753e 100644 --- a/docs/source-pytorch/extensions/callbacks.rst +++ b/docs/source-pytorch/extensions/callbacks.rst @@ -170,18 +170,6 @@ teardown .. automethod:: pytorch_lightning.callbacks.Callback.teardown :noindex: -on_init_start -^^^^^^^^^^^^^ - -.. automethod:: pytorch_lightning.callbacks.Callback.on_init_start - :noindex: - -on_init_end -^^^^^^^^^^^ - -.. automethod:: pytorch_lightning.callbacks.Callback.on_init_end - :noindex: - on_fit_start ^^^^^^^^^^^^ diff --git a/src/lightning_app/utilities/introspection.py b/src/lightning_app/utilities/introspection.py index 856f6d6ea84a8..53889708e2907 100644 --- a/src/lightning_app/utilities/introspection.py +++ b/src/lightning_app/utilities/introspection.py @@ -170,8 +170,6 @@ class LightningCallbackVisitor(LightningVisitor): methods: Set[str] = { "setup", "teardown", - "on_init_start", - "on_init_end", "on_fit_start", "on_fit_end", "on_sanity_check_start", diff --git a/src/pytorch_lightning/CHANGELOG.md b/src/pytorch_lightning/CHANGELOG.md index 3739bfcc42405..cb48712907d2d 100644 --- a/src/pytorch_lightning/CHANGELOG.md +++ b/src/pytorch_lightning/CHANGELOG.md @@ -221,6 +221,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Removed the deprecated `Trainer.use_amp` and `LightningModule.use_amp` attributes ([#14832](https://github.com/Lightning-AI/lightning/pull/14832)) +- Removed deprecated callback hooks + * `Callback.on_init_start` + * `Callback.on_init_end` + ### Fixed diff --git a/src/pytorch_lightning/callbacks/callback.py b/src/pytorch_lightning/callbacks/callback.py index cf57c5c2f7847..484be2213e2c5 100644 --- a/src/pytorch_lightning/callbacks/callback.py +++ b/src/pytorch_lightning/callbacks/callback.py @@ -78,22 +78,6 @@ def setup(self, trainer: "pl.Trainer", pl_module: "pl.LightningModule", stage: s def teardown(self, trainer: "pl.Trainer", pl_module: "pl.LightningModule", stage: str) -> None: """Called when fit, validate, test, predict, or tune ends.""" - def on_init_start(self, trainer: "pl.Trainer") -> None: - r""" - .. deprecated:: v1.6 - This callback hook was deprecated in v1.6 and will be removed in v1.8. - - Called when the trainer initialization begins, model has not yet been set. - """ - - def on_init_end(self, trainer: "pl.Trainer") -> None: - r""" - .. deprecated:: v1.6 - This callback hook was deprecated in v1.6 and will be removed in v1.8. - - Called when the trainer initialization ends, model has not yet been set. - """ - def on_fit_start(self, trainer: "pl.Trainer", pl_module: "pl.LightningModule") -> None: """Called when fit begins.""" diff --git a/src/pytorch_lightning/callbacks/lambda_function.py b/src/pytorch_lightning/callbacks/lambda_function.py index a37122cb2aa04..09d802ee8e78c 100644 --- a/src/pytorch_lightning/callbacks/lambda_function.py +++ b/src/pytorch_lightning/callbacks/lambda_function.py @@ -44,8 +44,6 @@ def __init__( setup: Optional[Callable] = None, on_configure_sharded_model: Optional[Callable] = None, teardown: Optional[Callable] = None, - on_init_start: Optional[Callable] = None, - on_init_end: Optional[Callable] = None, on_fit_start: Optional[Callable] = None, on_fit_end: Optional[Callable] = None, on_sanity_check_start: Optional[Callable] = None, diff --git a/src/pytorch_lightning/core/module.py b/src/pytorch_lightning/core/module.py index 428ed58ea7eda..5ae0fbd75034a 100644 --- a/src/pytorch_lightning/core/module.py +++ b/src/pytorch_lightning/core/module.py @@ -1193,10 +1193,6 @@ def configure_callbacks(self): early_stop = EarlyStopping(monitor="val_acc", mode="max") checkpoint = ModelCheckpoint(monitor="val_loss") return [early_stop, checkpoint] - - Note: - Certain callback methods like :meth:`~pytorch_lightning.callbacks.base.Callback.on_init_start` - will never be invoked on the new callbacks returned here. """ return [] diff --git a/src/pytorch_lightning/trainer/configuration_validator.py b/src/pytorch_lightning/trainer/configuration_validator.py index 796445d7da1ad..1d8c593496658 100644 --- a/src/pytorch_lightning/trainer/configuration_validator.py +++ b/src/pytorch_lightning/trainer/configuration_validator.py @@ -212,13 +212,6 @@ def _check_on_pretrain_routine(model: "pl.LightningModule") -> None: def _check_deprecated_callback_hooks(trainer: "pl.Trainer") -> None: for callback in trainer.callbacks: - if is_overridden(method_name="on_init_start", instance=callback): - rank_zero_deprecation( - "The `on_init_start` callback hook was deprecated in v1.6 and will be removed in v1.8." - ) - if is_overridden(method_name="on_init_end", instance=callback): - rank_zero_deprecation("The `on_init_end` callback hook was deprecated in v1.6 and will be removed in v1.8.") - if is_overridden(method_name="on_configure_sharded_model", instance=callback): rank_zero_deprecation( "The `on_configure_sharded_model` callback hook was deprecated in" diff --git a/src/pytorch_lightning/trainer/connectors/logger_connector/fx_validator.py b/src/pytorch_lightning/trainer/connectors/logger_connector/fx_validator.py index 87ff7428103b5..5e64d0705c8f2 100644 --- a/src/pytorch_lightning/trainer/connectors/logger_connector/fx_validator.py +++ b/src/pytorch_lightning/trainer/connectors/logger_connector/fx_validator.py @@ -57,8 +57,6 @@ class _LogOptions(TypedDict): "optimizer_zero_grad": _LogOptions( allowed_on_step=(False, True), allowed_on_epoch=(False, True), default_on_step=True, default_on_epoch=False ), - "on_init_start": None, - "on_init_end": None, "on_fit_start": None, "on_fit_end": None, "on_sanity_check_start": None, diff --git a/src/pytorch_lightning/trainer/trainer.py b/src/pytorch_lightning/trainer/trainer.py index ce0ed55d34c4c..4ea456b69a9a8 100644 --- a/src/pytorch_lightning/trainer/trainer.py +++ b/src/pytorch_lightning/trainer/trainer.py @@ -480,9 +480,6 @@ def __init__( accumulate_grad_batches, ) - # hook - self._call_callback_hooks("on_init_start") - # init data flags self.check_val_every_n_epoch: int self._data_connector.on_trainer_init( @@ -543,9 +540,6 @@ def __init__( num_sanity_val_steps, ) - # Callback system - self._call_callback_hooks("on_init_end") - def _init_debugging_flags( self, limit_train_batches: Optional[Union[int, float]], @@ -1488,15 +1482,6 @@ def _call_callback_hooks( **kwargs: Any, ) -> None: log.debug(f"{self.__class__.__name__}: calling callback hook: {hook_name}") - # TODO: remove if block in v1.8 - if hook_name in ("on_init_start", "on_init_end"): - # these `Callback` hooks are the only ones that do not take a lightning module. - # we also don't profile bc profiler hasn't been set yet - for callback in self.callbacks: - fn = getattr(callback, hook_name) - if callable(fn): - fn(self, *args, **kwargs) - return pl_module = self.lightning_module if pl_module: diff --git a/tests/tests_pytorch/callbacks/test_callbacks.py b/tests/tests_pytorch/callbacks/test_callbacks.py index a6b64d87fdec6..8f95631f68575 100644 --- a/tests/tests_pytorch/callbacks/test_callbacks.py +++ b/tests/tests_pytorch/callbacks/test_callbacks.py @@ -39,13 +39,8 @@ def configure_callbacks(self): ) def assert_expected_calls(_trainer, model_callback, trainer_callback): - # some methods in callbacks configured through model won't get called - uncalled_methods = [call.on_init_start(_trainer), call.on_init_end(_trainer)] - for uncalled in uncalled_methods: - assert uncalled not in model_callback.method_calls - # assert that the rest of calls are the same as for trainer callbacks - expected_calls = [m for m in trainer_callback.method_calls if m not in uncalled_methods] + expected_calls = [m for m in trainer_callback.method_calls if m] assert expected_calls assert model_callback.method_calls == expected_calls diff --git a/tests/tests_pytorch/callbacks/test_lambda_function.py b/tests/tests_pytorch/callbacks/test_lambda_function.py index 08fec1ebd2efb..782ac963205e4 100644 --- a/tests/tests_pytorch/callbacks/test_lambda_function.py +++ b/tests/tests_pytorch/callbacks/test_lambda_function.py @@ -48,10 +48,7 @@ def call(hook, *_, **__): limit_val_batches=1, callbacks=[LambdaCallback(**hooks_args)], ) - with pytest.deprecated_call( - match="`on_init_start` callback hook was deprecated in v1.6 and will be removed in v1.8." - ): - trainer.fit(model) + trainer.fit(model) ckpt_path = trainer.checkpoint_callback.best_model_path # raises KeyboardInterrupt and loads from checkpoint @@ -64,17 +61,8 @@ def call(hook, *_, **__): limit_predict_batches=1, callbacks=[LambdaCallback(**hooks_args)], ) - with pytest.deprecated_call( - match="`on_init_start` callback hook was deprecated in v1.6 and will be removed in v1.8." - ): - trainer.fit(model, ckpt_path=ckpt_path) - with pytest.deprecated_call( - match="`on_init_start` callback hook was deprecated in v1.6 and will be removed in v1.8." - ): - trainer.test(model) - with pytest.deprecated_call( - match="`on_init_start` callback hook was deprecated in v1.6 and will be removed in v1.8." - ): - trainer.predict(model) + trainer.fit(model, ckpt_path=ckpt_path) + trainer.test(model) + trainer.predict(model) assert checker == hooks diff --git a/tests/tests_pytorch/deprecated_api/test_remove_1-8.py b/tests/tests_pytorch/deprecated_api/test_remove_1-8.py index 7946dadaa9398..8d293ab3de5d9 100644 --- a/tests/tests_pytorch/deprecated_api/test_remove_1-8.py +++ b/tests/tests_pytorch/deprecated_api/test_remove_1-8.py @@ -28,34 +28,6 @@ from pytorch_lightning.trainer.states import RunningStage -def test_v1_8_0_on_init_start_end(tmpdir): - class TestCallback(Callback): - def on_init_start(self, trainer): - print("Starting to init trainer!") - - def on_init_end(self, trainer): - print("Trainer is init now") - - model = BoringModel() - - trainer = Trainer( - callbacks=[TestCallback()], - max_epochs=1, - fast_dev_run=True, - enable_progress_bar=False, - logger=False, - default_root_dir=tmpdir, - ) - with pytest.deprecated_call( - match="The `on_init_start` callback hook was deprecated in v1.6 and will be removed in v1.8" - ): - trainer.fit(model) - with pytest.deprecated_call( - match="The `on_init_end` callback hook was deprecated in v1.6 and will be removed in v1.8" - ): - trainer.validate(model) - - def test_v1_8_0_deprecated_call_hook(): trainer = Trainer( max_epochs=1, diff --git a/tests/tests_pytorch/models/test_hooks.py b/tests/tests_pytorch/models/test_hooks.py index 158371a3097c5..9f7c3394d5570 100644 --- a/tests/tests_pytorch/models/test_hooks.py +++ b/tests/tests_pytorch/models/test_hooks.py @@ -483,10 +483,6 @@ def training_step(self, batch, batch_idx): track_grad_norm=1, **kwargs, ) - assert called == [ - dict(name="Callback.on_init_start", args=(trainer,)), - dict(name="Callback.on_init_end", args=(trainer,)), - ] trainer.fit(model) saved_ckpt = { "callbacks": ANY, @@ -502,8 +498,6 @@ def training_step(self, batch, batch_idx): saved_ckpt[trainer.precision_plugin.__class__.__qualname__] = ANY device = torch.device("cuda:0" if "accelerator" in kwargs and kwargs["accelerator"] == "gpu" else "cpu") expected = [ - dict(name="Callback.on_init_start", args=(trainer,)), - dict(name="Callback.on_init_end", args=(trainer,)), dict(name="configure_callbacks"), dict(name="prepare_data"), dict(name="Callback.on_before_accelerator_backend_setup", args=(trainer, model)), @@ -600,10 +594,6 @@ def test_trainer_model_hook_system_fit_no_val_and_resume_max_epochs(tmpdir): callbacks=[callback], track_grad_norm=1, ) - assert called == [ - dict(name="Callback.on_init_start", args=(trainer,)), - dict(name="Callback.on_init_end", args=(trainer,)), - ] # resume from checkpoint with HookedModel model = HookedModel(called) @@ -620,8 +610,6 @@ def test_trainer_model_hook_system_fit_no_val_and_resume_max_epochs(tmpdir): } saved_ckpt = {**loaded_ckpt, "global_step": 4, "epoch": 1} expected = [ - dict(name="Callback.on_init_start", args=(trainer,)), - dict(name="Callback.on_init_end", args=(trainer,)), dict(name="configure_callbacks"), dict(name="prepare_data"), dict(name="Callback.on_before_accelerator_backend_setup", args=(trainer, model)), @@ -697,10 +685,6 @@ def test_trainer_model_hook_system_fit_no_val_and_resume_max_steps(tmpdir): callbacks=[callback], track_grad_norm=1, ) - assert called == [ - dict(name="Callback.on_init_start", args=(trainer,)), - dict(name="Callback.on_init_end", args=(trainer,)), - ] trainer.fit(model, ckpt_path=best_model_path) loaded_ckpt = { @@ -715,8 +699,6 @@ def test_trainer_model_hook_system_fit_no_val_and_resume_max_steps(tmpdir): } saved_ckpt = {**loaded_ckpt, "global_step": steps_after_reload} expected = [ - dict(name="Callback.on_init_start", args=(trainer,)), - dict(name="Callback.on_init_end", args=(trainer,)), dict(name="configure_callbacks"), dict(name="prepare_data"), dict(name="Callback.on_before_accelerator_backend_setup", args=(trainer, model)), @@ -778,10 +760,6 @@ def test_trainer_model_hook_system_eval(tmpdir, batches, verb, noun, dataloader, enable_model_summary=False, callbacks=[callback], ) - assert called == [ - dict(name="Callback.on_init_start", args=(trainer,)), - dict(name="Callback.on_init_end", args=(trainer,)), - ] fn = getattr(trainer, verb) fn(model, verbose=False) hooks = [ @@ -798,8 +776,6 @@ def test_trainer_model_hook_system_eval(tmpdir, batches, verb, noun, dataloader, dict(name=f"on_{noun}_model_train"), ] expected = [ - dict(name="Callback.on_init_start", args=(trainer,)), - dict(name="Callback.on_init_end", args=(trainer,)), dict(name="configure_callbacks"), dict(name="prepare_data"), dict(name="Callback.on_before_accelerator_backend_setup", args=(trainer, model)), @@ -822,14 +798,8 @@ def test_trainer_model_hook_system_predict(tmpdir): trainer = Trainer( default_root_dir=tmpdir, limit_predict_batches=batches, enable_progress_bar=False, callbacks=[callback] ) - assert called == [ - dict(name="Callback.on_init_start", args=(trainer,)), - dict(name="Callback.on_init_end", args=(trainer,)), - ] trainer.predict(model) expected = [ - dict(name="Callback.on_init_start", args=(trainer,)), - dict(name="Callback.on_init_end", args=(trainer,)), dict(name="configure_callbacks"), dict(name="prepare_data"), dict(name="Callback.on_before_accelerator_backend_setup", args=(trainer, model)), diff --git a/tests/tests_pytorch/trainer/logging_/test_logger_connector.py b/tests/tests_pytorch/trainer/logging_/test_logger_connector.py index 266710abf9434..533bbd4863de1 100644 --- a/tests/tests_pytorch/trainer/logging_/test_logger_connector.py +++ b/tests/tests_pytorch/trainer/logging_/test_logger_connector.py @@ -46,8 +46,6 @@ def test_fx_validator(): "on_fit_end", "on_configure_sharded_model", "on_fit_start", - "on_init_end", - "on_init_start", "on_exception", "on_load_checkpoint", "load_state_dict", @@ -90,8 +88,6 @@ def test_fx_validator(): "on_fit_end", "on_fit_start", "on_configure_sharded_model", - "on_init_end", - "on_init_start", "on_exception", "on_load_checkpoint", "load_state_dict", @@ -164,10 +160,6 @@ def call(hook, trainer=None, model=None, *_, **__): return lightning_module = trainer.lightning_module or model - if lightning_module is None: - # `on_init_{start,end}` do not have the `LightningModule` available - assert hook in ("on_init_start", "on_init_end") - return if hook in not_supported: with pytest.raises(MisconfigurationException, match=not_supported[hook]): From 47b866ff76a1d9107d1947024d010873210427e5 Mon Sep 17 00:00:00 2001 From: awaelchli Date: Fri, 23 Sep 2022 14:41:54 +0200 Subject: [PATCH 2/5] fix the test --- .../callbacks/test_lambda_function.py | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/tests/tests_pytorch/callbacks/test_lambda_function.py b/tests/tests_pytorch/callbacks/test_lambda_function.py index 782ac963205e4..d7816a4f3988a 100644 --- a/tests/tests_pytorch/callbacks/test_lambda_function.py +++ b/tests/tests_pytorch/callbacks/test_lambda_function.py @@ -48,7 +48,10 @@ def call(hook, *_, **__): limit_val_batches=1, callbacks=[LambdaCallback(**hooks_args)], ) - trainer.fit(model) + with pytest.deprecated_call( + match="`on_configure_sharded_model` callback hook was deprecated in v1.6 and will be removed in v1.8" + ): + trainer.fit(model) ckpt_path = trainer.checkpoint_callback.best_model_path # raises KeyboardInterrupt and loads from checkpoint @@ -61,8 +64,17 @@ def call(hook, *_, **__): limit_predict_batches=1, callbacks=[LambdaCallback(**hooks_args)], ) - trainer.fit(model, ckpt_path=ckpt_path) - trainer.test(model) - trainer.predict(model) + with pytest.deprecated_call( + match="`on_configure_sharded_model` callback hook was deprecated in v1.6 and will be removed in v1.8" + ): + trainer.fit(model, ckpt_path=ckpt_path) + with pytest.deprecated_call( + match="`on_configure_sharded_model` callback hook was deprecated in v1.6 and will be removed in v1.8" + ): + trainer.test(model) + with pytest.deprecated_call( + match="`on_configure_sharded_model` callback hook was deprecated in v1.6 and will be removed in v1.8" + ): + trainer.predict(model) assert checker == hooks From 93d93e4379aacfb0fa5a27173d206f8e75b24c1b Mon Sep 17 00:00:00 2001 From: awaelchli Date: Fri, 23 Sep 2022 14:44:35 +0200 Subject: [PATCH 3/5] remove unused import --- tests/tests_pytorch/callbacks/test_callbacks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tests_pytorch/callbacks/test_callbacks.py b/tests/tests_pytorch/callbacks/test_callbacks.py index 8f95631f68575..d8664c6a1b5b2 100644 --- a/tests/tests_pytorch/callbacks/test_callbacks.py +++ b/tests/tests_pytorch/callbacks/test_callbacks.py @@ -13,7 +13,7 @@ # limitations under the License. from pathlib import Path from re import escape -from unittest.mock import call, Mock +from unittest.mock import Mock import pytest From d3bc11db38cde2b8084ff4f4a465b844975c47b9 Mon Sep 17 00:00:00 2001 From: Andres Algaba Date: Sat, 24 Sep 2022 19:39:14 +0200 Subject: [PATCH 4/5] Update src/pytorch_lightning/CHANGELOG.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Adrian Wälchli --- src/pytorch_lightning/CHANGELOG.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/pytorch_lightning/CHANGELOG.md b/src/pytorch_lightning/CHANGELOG.md index cb48712907d2d..a13efb82d01bb 100644 --- a/src/pytorch_lightning/CHANGELOG.md +++ b/src/pytorch_lightning/CHANGELOG.md @@ -221,9 +221,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Removed the deprecated `Trainer.use_amp` and `LightningModule.use_amp` attributes ([#14832](https://github.com/Lightning-AI/lightning/pull/14832)) -- Removed deprecated callback hooks - * `Callback.on_init_start` - * `Callback.on_init_end` +- Removed the deprecated callback hooks `Callback.on_init_start` and `Callback.on_init_end` ([#14867](https://github.com/Lightning-AI/lightning/pull/14867)) ### Fixed From e9ee5fcf87238c7488ddd0e5d037edb98102382c Mon Sep 17 00:00:00 2001 From: awaelchli Date: Fri, 30 Sep 2022 13:55:18 +0200 Subject: [PATCH 5/5] reset changes in app/utilities/introspection this code needs to be compatible with old PL versions still --- src/lightning_app/utilities/introspection.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/lightning_app/utilities/introspection.py b/src/lightning_app/utilities/introspection.py index 53889708e2907..856f6d6ea84a8 100644 --- a/src/lightning_app/utilities/introspection.py +++ b/src/lightning_app/utilities/introspection.py @@ -170,6 +170,8 @@ class LightningCallbackVisitor(LightningVisitor): methods: Set[str] = { "setup", "teardown", + "on_init_start", + "on_init_end", "on_fit_start", "on_fit_end", "on_sanity_check_start",