Warn when self.log(..., logger=True) is called without a logger (#15814)

Author: carmocca

src/pytorch_lightning/CHANGELOG.md

@@ -27,6 +27,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Added a check to validate that wrapped FSDP models are used while initializing optimizers ([#15301](https://github.com/Lightning-AI/lightning/pull/15301))
 
 
+- Added a warning when `self.log(..., logger=True)` is called without a configured logger ([#15814](https://github.com/Lightning-AI/lightning/pull/15814))
+
 ### Changed
 
 - Drop PyTorch 1.9 support ([#15347](https://github.com/Lightning-AI/lightning/pull/15347))

src/pytorch_lightning/core/module.py

@@ -322,7 +322,7 @@ def log(
         name: str,
         value: _METRIC_COLLECTION,
         prog_bar: bool = False,
-        logger: bool = True,
+        logger: Optional[bool] = None,
         on_step: Optional[bool] = None,
         on_epoch: Optional[bool] = None,
         reduce_fx: Union[str, Callable] = "mean",
@@ -438,6 +438,16 @@ def log(
                 "With `def training_step(self, dataloader_iter)`, `self.log(..., batch_size=...)` should be provided."
             )
 
+        if logger and self.trainer.logger is None:
+            rank_zero_warn(
+                f"You called `self.log({name!r}, ..., logger=True)` but have no logger configured. You can enable one"
+                " by doing `Trainer(logger=ALogger(...))`"
+            )
+        if logger is None:
+            # we could set false here if there's no configured logger, however, we still need to compute the "logged"
+            # metrics anyway because that's what the evaluation loops use as return value
+            logger = True
+
         results.log(
             self._current_fx_name,
             name,
@@ -463,7 +473,7 @@ def log_dict(
         self,
         dictionary: Mapping[str, _METRIC_COLLECTION],
         prog_bar: bool = False,
-        logger: bool = True,
+        logger: Optional[bool] = None,
         on_step: Optional[bool] = None,
         on_epoch: Optional[bool] = None,
         reduce_fx: Union[str, Callable] = "mean",

tests/tests_pytorch/trainer/logging_/test_logger_connector.py

@@ -22,6 +22,7 @@
 from pytorch_lightning import LightningModule
 from pytorch_lightning.callbacks.callback import Callback
 from pytorch_lightning.demos.boring_classes import BoringModel, RandomDataset
+from pytorch_lightning.loggers import CSVLogger
 from pytorch_lightning.trainer import Trainer
 from pytorch_lightning.trainer.connectors.logger_connector.fx_validator import _FxValidator
 from pytorch_lightning.trainer.connectors.logger_connector.result import _ResultCollection
@@ -605,15 +606,25 @@ def test_result_collection_on_tensor_with_mean_reduction():
     }
 
 
-def test_logged_metrics_has_logged_epoch_value(tmpdir):
+@pytest.mark.parametrize("logger", (False, True))
+def test_logged_metrics_has_logged_epoch_value(tmpdir, logger):
     class TestModel(BoringModel):
         def training_step(self, batch, batch_idx):
             self.log("epoch", -batch_idx, logger=True)
             return super().training_step(batch, batch_idx)
 
     model = TestModel()
-    trainer = Trainer(default_root_dir=tmpdir, fast_dev_run=2)
-    trainer.fit(model)
+    trainer_kwargs = dict(
+        default_root_dir=tmpdir, limit_train_batches=2, limit_val_batches=0, max_epochs=1, logger=False
+    )
+    if logger:
+        trainer_kwargs["logger"] = CSVLogger(tmpdir)
+    trainer = Trainer(**trainer_kwargs)
+    if not logger:
+        with pytest.warns(match=r"log\('epoch', ..., logger=True\)` but have no logger"):
+            trainer.fit(model)
+    else:
+        trainer.fit(model)
 
     # should not get overridden if logged manually
     assert trainer.logged_metrics == {"epoch": -1}