Handling of KeyboardInterrupt

[mibaumgartner]

🐛 Bug

Same behavior as in #6807

May be caused due to the wrong indentation of the raise statement?
https://github.com/PyTorchLightning/pytorch-lightning/blob/75350938ca646efc0b4bac432ba2d5d4676662bb/pytorch_lightning/trainer/trainer.py#L524

To Reproduce

Run training and raise a Keyboard Interrupt.

Expected behavior

Training should end and KeyboardInterrupt should stop the whole program.

Environment

• PyTorch Lightning Version (e.g., 1.3.0):
• PyTorch Version (e.g., 1.8)
• Python version:
• OS (e.g., Linux):
• CUDA/cuDNN version:
• GPU models and configuration:
• How you installed PyTorch (conda, pip, source):
• If compiling from source, the output of torch.__config__.show():
• Any other relevant information:

Additional context

[tchaton]

@daniellepintz

[daniellepintz]

Thanks for reporting @mibaumgartner I am looking into it!

[daniellepintz]

May be caused due to the wrong indentation of the raise statement?

What do you mean exactly? I don't see any wrong indentation of the raise statement

[mibaumgartner]

Hi @daniellepintz, thanks for looking into this.
I thought about moving the raise statement one indentation to the left but it might be better (more explicit) to add a second raise statement in the keyboard interrupt block.

[daniellepintz]

@mibaumgartner Done in #9260

[ananthsub]

@tchaton @daniellepintz @awaelchli was it intentional that keyboard interrupt didn't re-raise the exception after doing the graceful teardown from #856 ? is that desirable?

[awaelchli]

yes certainly, ctrl+c -> graceful shutdown -> exit training without leaving behind a stack trace. that's simply for UX

[ananthsub]

yes certainly, ctrl+c -> graceful shutdown -> exit training without leaving behind a stack trace. that's simply for UX

Was there concern about ghost background processes in this case? Should this apply to all training types, or only for single device ones?

[awaelchli]

@daniellepintz interrupt / exception handling changes are not included in any release are they?
@mibaumgartner we are kindly requesting at least the PL version you were running this with please. KeyboardInterrupts work fine on master branch given my quick testing. more details would be appreciated to address your specific issue.

[daniellepintz]

yes that is correct, all of my recent changes are still unreleased

[mibaumgartner]

Hi @awaelchli,

It occurred with several (recent) versions of lightning, including the latest 1.4.5 release.

The Issue can be reproduced by raising a Keyboard Interrupt in the Lightning Module (could be a good test :) ):
https://colab.research.google.com/drive/1aSoLeHHJCyKCfxpm-YsdJ3Gq6YLNR2Ci?usp=sharing

Changes in notebook:

    def training_step(self, batch, batch_idx):
        output = self.layer(batch)
        loss = self.loss(batch, output)

        if batch_idx > 200: # see training starting, not actually needed
          raise KeyboardInterrupt

        return {"loss": loss}

    # Train the model ⚡
    trainer.fit(model, train, val)

    print("don't execute this since Keyboard Interrupt was raised in training")

    trainer.test(test_dataloaders=test)

(In the example notebook trainer.test also results in an additional error since the checkpoint from the training is missing)