Repository: Lightning-AI/pytorch-lightning
Pull request: Validate the model input of trainer methods
## What does this PR do?

Fixes #13355
Partially addresses #13981

When passing something other than a LightningModule to the Trainer, we currently get a misleading error message that doesn't help the user find the issue. Now I'm adding a check to all the Trainer methods that need a LightningModule. 


## Before submitting
- [x] Was this discussed/approved via a GitHub issue? (not for typos and docs)
- [x] Did you read the [contributor guideline](https://github.com/PyTorchLightning/pytorch-lightning/blob/master/.github/CONTRIBUTING.md), **Pull Request** section?
- [x] Did you make sure your PR does only one thing, instead of bundling different changes together?
- [x] Did you make sure to update the documentation with your changes? (if necessary)
- [x] Did you write any new necessary tests? (not for typos and docs)
- [x] Did you verify new and existing tests pass locally with your changes?
- [x] Did you update the [CHANGELOG](https://github.com/PyTorchLightning/pytorch-lightning/blob/master/CHANGELOG.md)? (not for typos, docs, test updates, or internal minor changes/refactorings)

<!-- For CHANGELOG separate each item in the unreleased section by a blank line to reduce collisions -->

## PR review
Anyone in the community is free to review the PR once the tests have passed.
Before you start reviewing make sure you have read [Review guidelines](https://github.com/PyTorchLightning/pytorch-lightning/wiki/Review-guidelines). In short, see the following bullet-list:

 - [x] Is this pull request ready for review? (if not, please submit in draft mode)
 - [x] Check that all items from **Before submitting** are resolved
 - [x] Make sure the title is self-explanatory and the description concisely explains the PR
 - [x] Add labels and milestones (and optionally projects) to the PR so it can be classified

## Did you have fun?
I made sure I had fun coding 🙃



cc @borda @justusschock @kaushikb11 @awaelchli @ananthsub @ninginthecloud @jjenniferdai @rohitgr7 @akihironitta

Problem statement:
TypeError: Unwrapping the module did not yield a `LightningModule`
I am following the tutorial of MNIST, and I passed a pretrained model from timm, and I got this error.

I am using default strategy and backend.

```py
`Traceback (most recent call last):
  File "train.py", line 172, in <module>
    train(args)
  File "train.py", line 163, in train
    trainer.fit(model=pl_model, train_dataloaders=train_loader, val_dataloaders=val_loader)
  File "/usr/local/lib/python3.7/dist-packages/pytorch_lightning/trainer/trainer.py", line 771, in fit
    self._fit_impl, model, train_dataloaders, val_dataloaders, datamodule, ckpt_path
  File "/usr/local/lib/python3.7/dist-packages/pytorch_lightning/trainer/trainer.py", line 721, in _call_and_handle_interrupt
    return self.strategy.launcher.launch(trainer_fn, *args, trainer=self, **kwargs)
  File "/usr/local/lib/python3.7/dist-packages/pytorch_lightning/strategies/launchers/spawn.py", line 82, in launch
    start_method=self._start_method,
  File "/usr/local/lib/python3.7/dist-packages/torch/multiprocessing/spawn.py", line 230, in spawn
    return start_processes(fn, args, nprocs, join, daemon, start_method='spawn')
  File "/usr/local/lib/python3.7/dist-packages/torch/multiprocessing/spawn.py", line 188, in start_processes
    while not context.join():
  File "/usr/local/lib/python3.7/dist-packages/torch/multiprocessing/spawn.py", line 150, in join
    raise ProcessRaisedException(msg, error_index, failed_process.pid)
torch.multiprocessing.spawn.ProcessRaisedException: 

-- Process 1 terminated with the following error:
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/dist-packages/torch/multiprocessing/spawn.py", line 59, in _wrap
    fn(i, *args)
  File "/usr/local/lib/python3.7/dist-packages/pytorch_lightning/strategies/launchers/spawn.py", line 101, in _wrapping_function
    results = function(*args, **kwargs)
  File "/usr/local/lib/python3.7/dist-packages/pytorch_lightning/trainer/trainer.py", line 811, in _fit_impl
    results = self._run(model, ckpt_path=self.ckpt_path)
  File "/usr/local/lib/python3.7/dist-packages/pytorch_lightning/trainer/trainer.py", line 1217, in _run
    self.strategy.setup(self)
  File "/usr/local/lib/python3.7/dist-packages/pytorch_lightning/strategies/ddp_spawn.py", line 141, in setup
    self.configure_ddp()
  File "/usr/local/lib/python3.7/dist-packages/pytorch_lightning/strategies/ddp_spawn.py", line 193, in configure_ddp
    self.setup_optimizers(self.lightning_module.trainer)
  File "/usr/local/lib/python3.7/dist-packages/pytorch_lightning/strategies/parallel.py", line 60, in lightning_module
    return unwrap_lightning_module(self.model) if self.model is not None else None
  File "/usr/local/lib/python3.7/dist-packages/pytorch_lightning/overrides/base.py", line 116, in unwrap_lightning_module
    raise TypeError(f"Unwrapping the module did not yield a `LightningModule`, got {type(model)} instead.")
TypeError: Unwrapping the module did not yield a `LightningModule`, got <class 'torch.nn.parallel.distributed.DistributedDataParallel'> instead.`
```

I am passing the model to pl.LightningModule as this:
```py
class PLModelFinetune(pl.LightningModule):
    def __init__(self, model, lr=1e-3):
        super().__init__()
        self.model = model
        self.lr = lr
        self.criterion = nn.CrossEntropyLoss(label_smoothing=0.1)
        # Metrics
        self.train_accuracy = torchmetrics.Accuracy()
        self.val_accuracy = torchmetrics.Accuracy()
        self.train_epoch_metrics = []
        self.val_epoch_metrics = []
```

cc @justusschock @awaelchli @ninginthecloud @rohitgr7 @kaushikb11 @akihironitta

Hints from issue discussion:
@ychenZHANG thanks for the issue. Could you maybe update your snippet a bit? What I am looking for is the class of the model (e.g which timm model you used) as well as your instantiation of the trainer. This would  make it significantly easier to debug this :)
I am having the same issue:
`model = BartForConditionalGeneration.from_pretrained("facebook/bart-base")`
`trainer = pl.Trainer(accelerator="gpu", devices=1,check_val_every_n_epoch=1, max_epochs=5)`

TypeError: Unwrapping the module did not yield a `LightningModule`, got <class 'transformers.models.bart.modeling_bart.BartForConditionalGeneration'> instead.


You are not by any chance trying to call Trainer.fit() with something not a LightningModule? Because that's not valid. The BartForConditionalGeneration you have here seems to be directly the model from huggingface transformers but that's not a LightningModule. 

Here is a simple repro that produces the same error:

```py
import torch
from pytorch_lightning import Trainer

model = torch.nn.Linear(2, 2)
trainer = Trainer()
trainer.fit(model) 
```

```
TypeError: Unwrapping the module did not yield a `LightningModule`, got <class 'torch.nn.modules.linear.Linear'> instead.
```

So the fix here for us is to update the input validation and messaging, so that it is clear to the user only LightningModules can be passed to the Trainer methods.

Commit message: Merge remote-tracking branch 'origin/bugfix/trainer-method-error' into bugfix/trainer-method-error

Patch to review:
diff --git a/src/pytorch_lightning/CHANGELOG.md b/src/pytorch_lightning/CHANGELOG.md
index baf01371fb8bc..3ea158d37fde8 100644
--- a/src/pytorch_lightning/CHANGELOG.md
+++ b/src/pytorch_lightning/CHANGELOG.md
@@ -170,6 +170,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Disallowed using `BatchSampler` when running on multiple IPUs ([#13854](https://github.com/PyTorchLightning/pytorch-lightning/pull/13854))
 
 
+- The `Trainer.{fit,validate,test,tune}` methods now raise a useful error message if the input is not a `LightningModule` ([#13892](https://github.com/Lightning-AI/lightning/pull/13892))
+
+
 ### Deprecated
 
 - Deprecated `pytorch_lightning.accelerators.gpu.GPUAccelerator` in favor of `pytorch_lightning.accelerators.cuda.CUDAAccelerator` ([#13636](https://github.com/Lightning-AI/lightning/pull/13636))
diff --git a/src/pytorch_lightning/trainer/trainer.py b/src/pytorch_lightning/trainer/trainer.py
index 561fe799f1010..01d13849f8126 100644
--- a/src/pytorch_lightning/trainer/trainer.py
+++ b/src/pytorch_lightning/trainer/trainer.py
@@ -696,6 +696,8 @@ def fit(
 
             datamodule: An instance of :class:`~pytorch_lightning.core.datamodule.LightningDataModule`.
         """
+        if not isinstance(model, pl.LightningModule):
+            raise TypeError(f"`Trainer.fit()` requires a `LightningModule`, got: {model.__class__.__qualname__}")
         self.strategy.model = model
         self._call_and_handle_interrupt(
             self._fit_impl, model, train_dataloaders, val_dataloaders, datamodule, ckpt_path
@@ -776,6 +778,8 @@ def validate(
             :meth:`~pytorch_lightning.core.module.LightningModule.validation_epoch_end`, etc.
             The length of the list corresponds to the number of validation dataloaders used.
         """
+        if model is not None and not isinstance(model, pl.LightningModule):
+            raise TypeError(f"`Trainer.validate()` requires a `LightningModule`, got: {model.__class__.__qualname__}")
         self.strategy.model = model or self.lightning_module
         return self._call_and_handle_interrupt(self._validate_impl, model, dataloaders, ckpt_path, verbose, datamodule)
 
@@ -864,6 +868,8 @@ def test(
             :meth:`~pytorch_lightning.core.module.LightningModule.test_epoch_end`, etc.
             The length of the list corresponds to the number of test dataloaders used.
         """
+        if model is not None and not isinstance(model, pl.LightningModule):
+            raise TypeError(f"`Trainer.test()` requires a `LightningModule`, got: {model.__class__.__qualname__}")
         self.strategy.model = model or self.lightning_module
         return self._call_and_handle_interrupt(self._test_impl, model, dataloaders, ckpt_path, verbose, datamodule)
 
@@ -951,6 +957,8 @@ def predict(
         Returns:
             Returns a list of dictionaries, one for each provided dataloader containing their respective predictions.
         """
+        if model is not None and not isinstance(model, pl.LightningModule):
+            raise TypeError(f"`Trainer.predict()` requires a `LightningModule`, got: {model.__class__.__qualname__}")
         self.strategy.model = model or self.lightning_module
         return self._call_and_handle_interrupt(
             self._predict_impl, model, dataloaders, datamodule, return_predictions, ckpt_path
@@ -1033,6 +1041,9 @@ def tune(
 
             lr_find_kwargs: Arguments for :func:`~pytorch_lightning.tuner.lr_finder.lr_find`
         """
+        if not isinstance(model, pl.LightningModule):
+            raise TypeError(f"`Trainer.tune()` requires a `LightningModule`, got: {model.__class__.__qualname__}")
+
         Trainer._log_api_event("tune")
 
         self.state.fn = TrainerFn.TUNING
diff --git a/tests/tests_pytorch/trainer/test_trainer.py b/tests/tests_pytorch/trainer/test_trainer.py
index ecc0ad724e879..f868dcc353e72 100644
--- a/tests/tests_pytorch/trainer/test_trainer.py
+++ b/tests/tests_pytorch/trainer/test_trainer.py
@@ -20,12 +20,14 @@
 from contextlib import nullcontext
 from copy import deepcopy
 from pathlib import Path
+from re import escape
 from unittest import mock
 from unittest.mock import ANY, call, patch
 
 import cloudpickle
 import pytest
 import torch
+import torch.nn as nn
 from torch.multiprocessing import ProcessRaisedException
 from torch.nn.parallel.distributed import DistributedDataParallel
 from torch.optim import SGD
@@ -71,6 +73,21 @@
     torch_test_assert_close = torch.testing.assert_allclose
 
 
+def test_trainer_error_when_input_not_lightning_module():
+    """Test that a useful error gets raised when the Trainer methods receive something other than a
+    LightningModule."""
+    trainer = Trainer()
+
+    for method in ("fit", "validate", "test", "predict"):
+        with pytest.raises(TypeError, match=escape(f"`Trainer.{method}()` requires a `LightningModule`, got: Linear")):
+            run_method = getattr(trainer, method)
+            run_method(nn.Linear(2, 2))
+
+    trainer = Trainer(auto_lr_find=True, auto_scale_batch_size=True)
+    with pytest.raises(TypeError, match=escape("`Trainer.tune()` requires a `LightningModule`, got: Linear")):
+        trainer.tune(nn.Linear(2, 2))
+
+
 @pytest.mark.parametrize("url_ckpt", [True, False])
 def test_no_val_module(monkeypatch, tmpdir, tmpdir_server, url_ckpt):
     """Tests use case where trainer saves the model, and user loads it from tags independently."""


Code review comments:
diff_hunk: @@ -170,6 +170,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Disallowed using `BatchSampler` when running on multiple IPUs ([#13854](https://github.com/PyTorchLightning/pytorch-lightning/pull/13854))
 
 
+- The `Trainer.{fit,validate,test,tune}` methods now raise a useful error message if the input is not a `LightningModule` ([#13892](https://github.com/Lightning-AI/lightning/pull/13892))
path: src/pytorch_lightning/CHANGELOG.md
text: @user1:
```suggestion
- The `Trainer.{fit,validate,test,predict,tune}` methods now raise a useful error message if the input is not a `LightningModule` ([#13892](https://github.com/Lightning-AI/lightning/pull/13892))
```