revert #9125 / fix back-compatibility with saving hparams as a whole container (#9642)

Author: awaelchli

CHANGELOG.md

@@ -5,12 +5,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 
 
-## [1.4.8] - 2021-09-21
+## [1.4.8] - 2021-09-22
 
 - Fixed error reporting in DDP process reconciliation when processes are launched by an external agent (#9389)
 - Added PL_RECONCILE_PROCESS environment variable to enable process reconciliation regardless of cluster environment settings (#9389)
 - Fixed `add_argparse_args` raising `TypeError` when args are typed as `typing.Generic` in Python 3.6 ([#9554](https://github.com/PyTorchLightning/pytorch-lightning/pull/9554))
-
+- Fixed back-compatibility for saving hyperparameters from a single container and inferring its argument name by reverting [#9125](https://github.com/PyTorchLightning/pytorch-lightning/pull/9125) ([#9642](https://github.com/PyTorchLightning/pytorch-lightning/pull/9642))
 
 ## [1.4.7] - 2021-09-14
 

pytorch_lightning/utilities/parsing.py

@@ -22,12 +22,8 @@
 from typing_extensions import Literal
 
 import pytorch_lightning as pl
-from pytorch_lightning.utilities import _OMEGACONF_AVAILABLE
 from pytorch_lightning.utilities.warnings import rank_zero_warn
 
-if _OMEGACONF_AVAILABLE:
-    from omegaconf.dictconfig import DictConfig
-
 
 def str_to_bool_or_str(val: str) -> Union[str, bool]:
     """Possibly convert a string representation of truth to bool.
@@ -208,57 +204,46 @@ def save_hyperparameters(
     obj: Any, *args: Any, ignore: Optional[Union[Sequence[str], str]] = None, frame: Optional[types.FrameType] = None
 ) -> None:
     """See :meth:`~pytorch_lightning.LightningModule.save_hyperparameters`"""
-    hparams_container_types = [Namespace, dict]
-    if _OMEGACONF_AVAILABLE:
-        hparams_container_types.append(DictConfig)
-    # empty container
+
     if len(args) == 1 and not isinstance(args, str) and not args[0]:
+        # args[0] is an empty container
         return
-    # container
-    elif len(args) == 1 and isinstance(args[0], tuple(hparams_container_types)):
-        hp = args[0]
-        obj._hparams_name = "hparams"
-        obj._set_hparams(hp)
-        obj._hparams_initial = copy.deepcopy(obj._hparams)
-        return
-    # non-container args parsing
+
+    if not frame:
+        current_frame = inspect.currentframe()
+        # inspect.currentframe() return type is Optional[types.FrameType]: current_frame.f_back called only if available
+        if current_frame:
+            frame = current_frame.f_back
+    if not isinstance(frame, types.FrameType):
+        raise AttributeError("There is no `frame` available while being required.")
+
+    if is_dataclass(obj):
+        init_args = {f.name: getattr(obj, f.name) for f in fields(obj)}
     else:
-        if not frame:
-            current_frame = inspect.currentframe()
-            # inspect.currentframe() return type is Optional[types.FrameType]
-            # current_frame.f_back called only if available
-            if current_frame:
-                frame = current_frame.f_back
-        if not isinstance(frame, types.FrameType):
-            raise AttributeError("There is no `frame` available while being required.")
-
-        if is_dataclass(obj):
-            init_args = {f.name: getattr(obj, f.name) for f in fields(obj)}
-        else:
-            init_args = get_init_args(frame)
-        assert init_args, f"failed to inspect the obj init - {frame}"
-
-        if ignore is not None:
-            if isinstance(ignore, str):
-                ignore = [ignore]
-            if isinstance(ignore, (list, tuple, set)):
-                ignore = [arg for arg in ignore if isinstance(arg, str)]
-            init_args = {k: v for k, v in init_args.items() if k not in ignore}
-
-        if not args:
-            # take all arguments
-            hp = init_args
-            obj._hparams_name = "kwargs" if hp else None
+        init_args = get_init_args(frame)
+    assert init_args, "failed to inspect the obj init"
+
+    if ignore is not None:
+        if isinstance(ignore, str):
+            ignore = [ignore]
+        if isinstance(ignore, (list, tuple)):
+            ignore = [arg for arg in ignore if isinstance(arg, str)]
+        init_args = {k: v for k, v in init_args.items() if k not in ignore}
+
+    if not args:
+        # take all arguments
+        hp = init_args
+        obj._hparams_name = "kwargs" if hp else None
+    else:
+        # take only listed arguments in `save_hparams`
+        isx_non_str = [i for i, arg in enumerate(args) if not isinstance(arg, str)]
+        if len(isx_non_str) == 1:
+            hp = args[isx_non_str[0]]
+            cand_names = [k for k, v in init_args.items() if v == hp]
+            obj._hparams_name = cand_names[0] if cand_names else None
         else:
-            # take only listed arguments in `save_hparams`
-            isx_non_str = [i for i, arg in enumerate(args) if not isinstance(arg, str)]
-            if len(isx_non_str) == 1:
-                hp = args[isx_non_str[0]]
-                cand_names = [k for k, v in init_args.items() if v == hp]
-                obj._hparams_name = cand_names[0] if cand_names else None
-            else:
-                hp = {arg: init_args[arg] for arg in args if isinstance(arg, str)}
-                obj._hparams_name = "kwargs"
+            hp = {arg: init_args[arg] for arg in args if isinstance(arg, str)}
+            obj._hparams_name = "kwargs"
 
     # `hparams` are expected here
     if hp:

tests/models/test_hparams.py

@@ -689,6 +689,14 @@ def test_empty_hparams_container(tmpdir):
     assert not model.hparams
 
 
+def test_hparams_name_from_container(tmpdir):
+    """Test that save_hyperparameters(container) captures the name of the argument correctly."""
+    model = HparamsKwargsContainerModel(a=1, b=2)
+    assert model._hparams_name is None
+    model = HparamsNamespaceContainerModel(Namespace(a=1, b=2))
+    assert model._hparams_name == "config"
+
+
 @dataclass
 class DataClassModel(BoringModel):