Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate on_keyboard_interrupt callback hook #9260

Merged
merged 22 commits into from
Sep 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
8ce82a3
add on_exception callback hook
daniellepintz Aug 28, 2021
cddb358
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 29, 2021
7e16fe9
small fixes/lints
daniellepintz Aug 29, 2021
7fb01c0
Merge branch 'on_exception' of github.com:daniellepintz/pytorch-light…
daniellepintz Aug 29, 2021
58318c3
separate prs
daniellepintz Aug 29, 2021
b97c668
deprecate on_keyboard_interrupt
daniellepintz Aug 29, 2021
a858faf
Merge branch 'master' of https://github.com/PyTorchLightning/pytorch-…
daniellepintz Sep 1, 2021
3c71e18
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 1, 2021
99c85b6
small fix
daniellepintz Sep 1, 2021
2bdfec2
Merge branch 'dep_keyboard' of github.com:daniellepintz/pytorch-light…
daniellepintz Sep 1, 2021
b8b4de2
small fix
daniellepintz Sep 1, 2021
7698965
Apply suggestions from code review
Borda Sep 1, 2021
a6a45fc
note
Borda Sep 1, 2021
3ced67d
fix failing tests
daniellepintz Sep 2, 2021
b618196
Merge branch 'dep_keyboard' of github.com:daniellepintz/pytorch-light…
daniellepintz Sep 2, 2021
c6fdb6a
raise keyboardinterrupt
daniellepintz Sep 2, 2021
97b53f1
fix tests
daniellepintz Sep 2, 2021
f98ea3f
fix test
daniellepintz Sep 2, 2021
26f484e
Delete cluster
daniellepintz Sep 2, 2021
cef8d4d
update changelog
daniellepintz Sep 2, 2021
a6299a8
Merge branch 'dep_keyboard' of github.com:daniellepintz/pytorch-light…
daniellepintz Sep 2, 2021
21b8abe
change raise exception to raise
daniellepintz Sep 6, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -167,12 +167,16 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).

- Deprecated the `TestTubeLogger` ([#9065](https://github.com/PyTorchLightning/pytorch-lightning/pull/9065))


- Deprecated `on_{train/val/test/predict}_dataloader()` from `LightningModule` and `LightningDataModule` [#9098](https://github.com/PyTorchLightning/pytorch-lightning/pull/9098)


- Updated deprecation of `argparse_utils.py` from removal in 1.4 to 2.0 ([#9162](https://github.com/PyTorchLightning/pytorch-lightning/pull/9162))


- Deprecated `on_keyboard_interrupt` callback hook in favor of new `on_exception` hook ([#9260](https://github.com/PyTorchLightning/pytorch-lightning/pull/9260))


- Deprecated passing `process_position` to the `Trainer` constructor in favor of adding the `ProgressBar` callback with `process_position` directly to the list of callbacks ([#9222](https://github.com/PyTorchLightning/pytorch-lightning/pull/9222))


Expand Down
7 changes: 6 additions & 1 deletion pytorch_lightning/callbacks/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,12 @@ def on_predict_end(self, trainer: "pl.Trainer", pl_module: "pl.LightningModule")
pass

def on_keyboard_interrupt(self, trainer: "pl.Trainer", pl_module: "pl.LightningModule") -> None:
"""Called when the training is interrupted by ``KeyboardInterrupt``."""
r"""
.. deprecated:: v1.5
This callback hook was deprecated in v1.5 in favor of `on_exception` and will be removed in v1.7.

Called when any trainer execution is interrupted by KeyboardInterrupt.
"""
pass

def on_exception(self, trainer: "pl.Trainer", pl_module: "pl.LightningModule", exception: BaseException) -> None:
Expand Down
7 changes: 6 additions & 1 deletion pytorch_lightning/trainer/callback_hook.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,12 @@ def on_predict_end(self) -> None:
callback.on_predict_end(self, self.lightning_module)

def on_keyboard_interrupt(self):
"""Called when the training is interrupted by KeyboardInterrupt."""
r"""
.. deprecated:: v1.5
This callback hook was deprecated in v1.5 in favor of `on_exception` and will be removed in v1.7.

Called when any trainer execution is interrupted by KeyboardInterrupt.
"""
for callback in self.callbacks:
callback.on_keyboard_interrupt(self, self.lightning_module)

Expand Down
11 changes: 11 additions & 0 deletions pytorch_lightning/trainer/configuration_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ def verify_loop_configurations(self, model: "pl.LightningModule") -> None:
elif self.trainer.state.fn == TrainerFn.PREDICTING:
self.__verify_predict_loop_configuration(model)
self.__verify_dp_batch_transfer_support(model)
# TODO: Delete _check_on_keyboard_interrupt in v1.7
self._check_on_keyboard_interrupt()

def __verify_train_loop_configuration(self, model: "pl.LightningModule") -> None:
# -----------------------------------
Expand Down Expand Up @@ -201,3 +203,12 @@ def __check_training_step_requires_dataloader_iter(self, model: "pl.LightningMod
"The model taking a `dataloader_iter` argument in your `training_step` "
"is incompatible with `truncated_bptt_steps > 0`."
)

def _check_on_keyboard_interrupt(self) -> None:
"""Checks if on_keyboard_interrupt is overriden and sends a deprecation warning."""
for callback in self.trainer.callbacks:
if is_overridden(method_name="on_keyboard_interrupt", instance=callback):
rank_zero_deprecation(
"The `on_keyboard_interrupt` callback hook was deprecated in v1.5 and will be removed in v1.7."
" Please use the `on_exception` callback hook instead."
)
1 change: 1 addition & 0 deletions pytorch_lightning/trainer/trainer.py
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,7 @@ def _call_and_handle_interrupt(self, trainer_fn: Callable, *args: Any, **kwargs:
"""
try:
return trainer_fn(*args, **kwargs)
# TODO: treat KeyboardInterrupt as BaseException (delete the code below) in v1.7
except KeyboardInterrupt as exception:
rank_zero_warn("Detected KeyboardInterrupt, attempting graceful shutdown...")
# user could press Ctrl+c many times... only shutdown once
Expand Down
2 changes: 2 additions & 0 deletions pytorch_lightning/utilities/model_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ def is_overridden(
parent = pl.LightningModule
elif isinstance(instance, pl.LightningDataModule):
parent = pl.LightningDataModule
elif isinstance(instance, pl.Callback):
parent = pl.Callback
if parent is None:
raise ValueError("Expected a parent")

Expand Down
6 changes: 3 additions & 3 deletions tests/callbacks/test_callbacks.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@
def test_callbacks_configured_in_model(tmpdir):
"""Test the callback system with callbacks added through the model hook."""

model_callback_mock = Mock()
trainer_callback_mock = Mock()
model_callback_mock = Mock(spec=Callback, model=Callback())
trainer_callback_mock = Mock(spec=Callback, model=Callback())

class TestModel(BoringModel):
def configure_callbacks(self):
Expand Down Expand Up @@ -79,7 +79,7 @@ def assert_expected_calls(_trainer, model_callback, trainer_callback):

def test_configure_callbacks_hook_multiple_calls(tmpdir):
"""Test that subsequent calls to `configure_callbacks` do not change the callbacks list."""
model_callback_mock = Mock()
model_callback_mock = Mock(spec=Callback, model=Callback())

class TestModel(BoringModel):
def configure_callbacks(self):
Expand Down
25 changes: 24 additions & 1 deletion tests/deprecated_api/test_remove_1-7.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import pytest

from pytorch_lightning import LightningDataModule, Trainer
from pytorch_lightning import Callback, LightningDataModule, Trainer
from pytorch_lightning.loggers import TestTubeLogger
from tests.deprecated_api import _soft_unimport_module
from tests.helpers import BoringModel
Expand Down Expand Up @@ -118,6 +118,29 @@ def test_v1_7_0_test_tube_logger(_, tmpdir):
_ = TestTubeLogger(tmpdir)


def test_v1_7_0_on_interrupt(tmpdir):
class HandleInterruptCallback(Callback):
def on_keyboard_interrupt(self, trainer, pl_module):
print("keyboard interrupt")

model = BoringModel()
handle_interrupt_callback = HandleInterruptCallback()

trainer = Trainer(
callbacks=[handle_interrupt_callback],
max_epochs=1,
limit_val_batches=0.1,
limit_train_batches=0.2,
progress_bar_refresh_rate=0,
logger=False,
default_root_dir=tmpdir,
)
with pytest.deprecated_call(
match="The `on_keyboard_interrupt` callback hook was deprecated in v1.5 and will be removed in v1.7"
):
trainer.fit(model)


def test_v1_7_0_process_position_trainer_constructor(tmpdir):
with pytest.deprecated_call(match=r"Setting `Trainer\(process_position=5\)` is deprecated in v1.5"):
_ = Trainer(process_position=5)