Repository: Lightning-AI/pytorch-lightning

Issue:
Logging Hyperparameters for list of dicts
### Bug description

Currently, when hyper parameters are logged with `log_hyperparams` the function calls [_flatten_dict](https://github.com/Lightning-AI/pytorch-lightning/blob/master/src/lightning/fabric/utilities/logger.py#L75-L106) to collapse the dict to a single level. However, when the config contains a list of dicts, this gets flattened to a single string. Instead I would propose to log the list as `[key/0/item, key/1/item]` etc.

A fix could be simple:

```python
def _flatten_dict(params: MutableMapping[Any, Any], delimiter: str = "/", parent_key: str = "") -> Dict[str, Any]:
    """Flatten hierarchical dict, e.g. ``{'a': {'b': 'c'}} -> {'a/b': 'c'}``.

    Args:
        params: Dictionary containing the hyperparameters
        delimiter: Delimiter to express the hierarchy. Defaults to ``'/'``.

    Returns:
        Flattened dict.

    Examples:
        >>> _flatten_dict({'a': {'b': 'c'}})
        {'a/b': 'c'}
        >>> _flatten_dict({'a': {'b': 123}})
        {'a/b': 123}
        >>> _flatten_dict({5: {'a': 123}})
        {'5/a': 123}
        >>> _flatten_dict({"dl": [{"a": 1, "c": 3}, {"b": 2, "d": 5}], "l": [1, 2, 3, 4]})
        {'dl/0/a': 1, 'dl/0/c': 3, 'dl/1/b': 2, 'dl/1/d': 5, 'l': [1, 2, 3, 4]}

    """
    result: Dict[str, Any] = {}
    for k, v in params.items():
        new_key = parent_key + delimiter + str(k) if parent_key else str(k)
        if is_dataclass(v):
            v = asdict(v)
        elif isinstance(v, Namespace):
            v = vars(v)

        if isinstance(v, MutableMapping):
            result = {**result, **_flatten_dict(v, parent_key=new_key, delimiter=delimiter)}
        # Also handle the case where v is a list of dictionaries
        elif isinstance(v, list) and all(isinstance(item, MutableMapping) for item in v):
            for i, item in enumerate(v):
                result = {**result, **_flatten_dict(item, parent_key=f"{new_key}/{i}", delimiter=delimiter)}
        else:
            result[new_key] = v
    return result
```

### What version are you seeing the problem on?

master

### How to reproduce the bug

_No response_

### Error messages and logs

```
# Error messages and logs here please
```


### Environment

<details>
  <summary>Current environment</summary>

```
#- Lightning Component (e.g. Trainer, LightningModule, LightningApp, LightningWork, LightningFlow):
#- PyTorch Lightning Version (e.g., 1.5.0):
#- Lightning App Version (e.g., 0.5.2):
#- PyTorch Version (e.g., 2.0):
#- Python version (e.g., 3.9):
#- OS (e.g., Linux):
#- CUDA/cuDNN version:
#- GPU models and configuration:
#- How you installed Lightning(`conda`, `pip`, source):
#- Running environment of LightningApp (e.g. local, cloud):
```

</details>


### More info

_No response_

Discussion:

Solution patch:
diff --git a/src/lightning/fabric/CHANGELOG.md b/src/lightning/fabric/CHANGELOG.md
index 992dbbb4f..a8d5d825e 100644
--- a/src/lightning/fabric/CHANGELOG.md
+++ b/src/lightning/fabric/CHANGELOG.md
@@ -6,9 +6,15 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 
 ## [unreleased] - YYYY-MM-DD
 
+### Added
+
+- Added logging support for list of dicts without collapsing to a single key ([#19957](https://github.com/Lightning-AI/pytorch-lightning/issues/19957))
+
+
 ### Removed
 
-Removed legacy supoport for `lightning run model`. Use `fabric run` instead. ([#20588](https://github.com/Lightning-AI/pytorch-lightning/pull/20588))
+- Removed legacy supoport for `lightning run model`. Use `fabric run` instead. ([#20588](https://github.com/Lightning-AI/pytorch-lightning/pull/20588))
+
 
 ## [2.5.0] - 2024-12-19
 
diff --git a/src/lightning/fabric/utilities/logger.py b/src/lightning/fabric/utilities/logger.py
index dd2b0a366..04b9069dd 100644
--- a/src/lightning/fabric/utilities/logger.py
+++ b/src/lightning/fabric/utilities/logger.py
@@ -91,6 +91,8 @@ def _flatten_dict(params: MutableMapping[Any, Any], delimiter: str = "/", parent
         {'a/b': 123}
         >>> _flatten_dict({5: {'a': 123}})
         {'5/a': 123}
+        >>> _flatten_dict({"dl": [{"a": 1, "c": 3}, {"b": 2, "d": 5}], "l": [1, 2, 3, 4]})
+        {'dl/0/a': 1, 'dl/0/c': 3, 'dl/1/b': 2, 'dl/1/d': 5, 'l': [1, 2, 3, 4]}
 
     """
     result: dict[str, Any] = {}
@@ -103,6 +105,10 @@ def _flatten_dict(params: MutableMapping[Any, Any], delimiter: str = "/", parent
 
         if isinstance(v, MutableMapping):
             result = {**result, **_flatten_dict(v, parent_key=new_key, delimiter=delimiter)}
+        # Also handle the case where v is a list of dictionaries
+        elif isinstance(v, list) and all(isinstance(item, MutableMapping) for item in v):
+            for i, item in enumerate(v):
+                result = {**result, **_flatten_dict(item, parent_key=f"{new_key}/{i}", delimiter=delimiter)}
         else:
             result[new_key] = v
     return result
diff --git a/src/lightning/pytorch/trainer/trainer.py b/src/lightning/pytorch/trainer/trainer.py
index 8e119e244..6c3ff4612 100644
--- a/src/lightning/pytorch/trainer/trainer.py
+++ b/src/lightning/pytorch/trainer/trainer.py
@@ -542,6 +542,7 @@ class Trainer:
         self.state.fn = TrainerFn.FITTING
         self.state.status = TrainerStatus.RUNNING
         self.training = True
+        self.should_stop = False
         call._call_and_handle_interrupt(
             self, self._fit_impl, model, train_dataloaders, val_dataloaders, datamodule, ckpt_path
         )


Test patch:
diff --git a/tests/tests_fabric/utilities/test_logger.py b/tests/tests_fabric/utilities/test_logger.py
index 268231431..4cb55c4cb 100644
--- a/tests/tests_fabric/utilities/test_logger.py
+++ b/tests/tests_fabric/utilities/test_logger.py
@@ -64,6 +64,12 @@ def test_flatten_dict():
     assert params["c/8"] == "foo"
     assert params["c/9/10"] == "bar"
 
+    # Test list of nested dicts flattening
+    params = {"dl": [{"a": 1, "c": 3}, {"b": 2, "d": 5}], "l": [1, 2, 3, 4]}
+    params = _flatten_dict(params)
+
+    assert params == {"dl/0/a": 1, "dl/0/c": 3, "dl/1/b": 2, "dl/1/d": 5, "l": [1, 2, 3, 4]}
+
     # Test flattening of argparse Namespace
     params = Namespace(a=1, b=2)
     wrapping_dict = {"params": params}
diff --git a/tests/tests_pytorch/loops/test_training_epoch_loop.py b/tests/tests_pytorch/loops/test_training_epoch_loop.py
index 71252fe20..35a9dacae 100644
--- a/tests/tests_pytorch/loops/test_training_epoch_loop.py
+++ b/tests/tests_pytorch/loops/test_training_epoch_loop.py
@@ -19,7 +19,7 @@ import torch
 from lightning_utilities.test.warning import no_warning_call
 
 from lightning.fabric.utilities.warnings import PossibleUserWarning
-from lightning.pytorch.callbacks import ModelCheckpoint
+from lightning.pytorch.callbacks import EarlyStopping, ModelCheckpoint
 from lightning.pytorch.demos.boring_classes import BoringModel
 from lightning.pytorch.trainer.trainer import Trainer
 
@@ -92,7 +92,16 @@ def test_should_stop_triggers_validation_once(min_epochs, min_steps, val_count,
     (min_epochs/steps is satisfied).
 
     """
-    model = BoringModel()
+
+    class NewBoring(BoringModel):
+        def training_step(self, batch, batch_idx):
+            self.log("loss", self.step(batch))
+            return {"loss": self.step(batch)}
+
+    model = NewBoring()
+    # create a stopping condition with a high threshold so it triggers immediately
+    # check the condition before validation so the count is unaffected
+    stopping = EarlyStopping(monitor="loss", check_on_train_epoch_end=True, stopping_threshold=100)
     trainer = Trainer(
         default_root_dir=tmp_path,
         num_sanity_val_steps=0,
@@ -103,8 +112,8 @@ def test_should_stop_triggers_validation_once(min_epochs, min_steps, val_count,
         min_steps=min_steps,
         enable_model_summary=False,
         enable_checkpointing=False,
+        callbacks=[stopping],
     )
-    trainer.should_stop = True  # Request to stop before min_epochs/min_steps are reached
     trainer.fit_loop.epoch_loop.val_loop.run = Mock()
     trainer.fit(model)
     assert trainer.fit_loop.epoch_loop.val_loop.run.call_count == val_count