Fix save_hyperparameters ignore precedence in subclasses (#21490)

Author: aditya0by0

src/lightning/pytorch/CHANGELOG.md

@@ -26,6 +26,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 
 ### Fixed
 
+- Fixed `save_hyperparameters(ignore=...)` behavior so subclass ignore rules override base class rules (#[21490](https://github.com/Lightning-AI/pytorch-lightning/pull/21490))
+
+
 - Fixed `LightningDataModule.load_from_checkpoint` to restore the datamodule subclass and hyperparameters ([#21478](https://github.com/Lightning-AI/pytorch-lightning/pull/21478))
 
 

src/lightning/pytorch/core/mixins/hparams_mixin.py

@@ -138,6 +138,19 @@ def _set_hparams(self, hp: Union[MutableMapping, Namespace, str]) -> None:
         else:
             self._hparams = hp
 
+    def remove_ignored_hparams(self, ignore_list: list[str]) -> None:
+        """Remove ignored hyperparameters from the stored state.
+
+        This allows derived classes to drop hyperparameters previously saved
+        by base classes.
+
+        Args:
+            ignore_list: Names of hyperparameters to remove.
+
+        """
+        for key in ignore_list:
+            self._hparams.pop(key, None)
+
     @staticmethod
     def _to_hparams_dict(hp: Union[MutableMapping, Namespace, str]) -> Union[MutableMapping, AttributeDict]:
         if isinstance(hp, Namespace):

src/lightning/pytorch/utilities/parsing.py

@@ -204,6 +204,9 @@ def save_hyperparameters(
 
     # `hparams` are expected here
     obj._set_hparams(hp)
+    # Remove ignored hparams from the stored hyperparameters.
+    # Allows a derived class to drop hparams previously saved by a base class.
+    obj.remove_ignored_hparams(ignore)
 
     for k, v in obj._hparams.items():
         if isinstance(v, nn.Module):

tests/tests_pytorch/models/test_hparams.py

@@ -838,7 +838,9 @@ def __init__(self, arg1, arg2, arg3):
 
     # test proper property assignments
     assert model.hparams.arg1 == 14
-    for arg in ignore:
+
+    ignore_args = ignore if isinstance(ignore, (list, tuple)) else [ignore]
+    for arg in ignore_args:
         assert arg not in model.hparams
 
     # verify we can train
@@ -854,7 +856,32 @@ def __init__(self, arg1, arg2, arg3):
     # verify that model loads correctly
     model = LocalModel.load_from_checkpoint(raw_checkpoint_path, arg2=123, arg3=100)
     assert model.hparams.arg1 == 14
-    for arg in ignore:
+    for arg in ignore_args:
+        assert arg not in model.hparams
+
+
+@pytest.mark.parametrize("ignore", ["arg2", ("arg2", "arg3")])
+def test_hparams_ignore_in_subclass_overrides_base(tmp_path, ignore):
+    """Test that hyperparameters can be ignored when `save_hyperparameters` is called in both a base class and a
+    subclass, and that ignore rules defined in the subclass override hyperparameters saved by the base class."""
+
+    class BaseBoringModel(BoringModel):
+        def __init__(self, arg1, arg2, arg3):
+            super().__init__()
+            self.save_hyperparameters(ignore="arg1")
+
+    class LocalModel(BaseBoringModel):
+        def __init__(self, arg1, arg2, arg3):
+            super().__init__(arg1=arg1, arg2=arg2, arg3=arg3)
+            self.save_hyperparameters(ignore=ignore)
+
+    model = LocalModel(arg1=14, arg2=90, arg3=50)
+
+    # `arg1` was ignored by the base class,
+    # but the subclass did not ignore it, so it should be present.
+    assert model.hparams.arg1 == 14
+    ignore_args = ignore if isinstance(ignore, (list, tuple)) else [ignore]
+    for arg in ignore_args:
         assert arg not in model.hparams