-
-
Notifications
You must be signed in to change notification settings - Fork 60
Closed
Labels
bugSomething isn't workingSomething isn't working
Description
🕰️ Regression report
Up to jsonargparse 4.38, the argument defaults for a subclass (e.g. using LightningCLI's subclass_mode_model=True) were correctly taken from the definition of the subclass.
Since jsonargparse 4.39, the argument defaults of the parent class are used instead. This breaks existing code that depend on the default values, or perform validation on them.
To reproduce
With this Python file:
# /usr/bin/env python
# regression.py
from lightning.pytorch.cli import LightningCLI
from lightning.pytorch.demos.boring_classes import BoringModel
class MyBaseModel(BoringModel):
def __init__(
self,
loss: str = "MSELoss",
) -> None:
super().__init__()
self.validate_loss(loss)
def validate_loss(self, loss: str) -> None:
# Allow everything.
pass
class MySubModel(MyBaseModel):
def __init__(
self,
loss: str = "BCEWithLogitsLoss",
) -> None:
super().__init__(loss=loss)
def validate_loss(self, loss: str) -> None:
allowed_loss_functions = ["BCEWithLogitsLoss", "BCELoss", "CosineEmbeddingLoss"]
if loss not in allowed_loss_functions:
raise ValueError(
f"Loss function {loss} is not applicable for model MySubModel. "
f"Applicable loss functions are: {allowed_loss_functions}"
)
if __name__ == "__main__":
cli = LightningCLI(MyBaseModel, subclass_mode_model=True)And this script:
#!/bin/bash
# regression.sh
python regression.py fit --model MySubModel --trainer.fast_dev_run=TrueGit bisect
git bisect output:
ce4b8aa8f26cc91b0878329b8c14f1b7177f1774 is the first bad commit
commit ce4b8aa8f26cc91b0878329b8c14f1b7177f1774
Author: Mauricio Villegas <5780272+mauvilsa@users.noreply.github.com>
Date: Tue Apr 22 07:54:46 2025 +0200
Fix list append nested in subclass not working (#710)
CHANGELOG.rst | 2 ++
jsonargparse/_actions.py | 2 +-
jsonargparse/_core.py | 18 +++++++++++++-----
jsonargparse/_typehints.py | 20 +++++++++-----------
jsonargparse_tests/test_subclasses.py | 12 +++++++++---
jsonargparse_tests/test_typehints.py | 26 ++++++++++++++++++++++++++
6 files changed, 60 insertions(+), 20 deletions(-)
bisect found first bad commit
With jsonargparse <= 4.38
Run succeeds:
$ ./regression.sh
# Successful runCorrect config is printed:
$ python regression.py fit --model MyBaseModel --print_config
...
model:
class_path: __main__.MyBaseModel
init_args:
loss: MSELoss
...
$ python regression.py fit --model MySubModel --print_config
...
model:
class_path: __main__.MySubModel
init_args:
loss: BCEWithLogitsLoss # correct default value
...With jsonargparse >= 4.39
Run fails:
$ ./regression.sh
...
Traceback (most recent call last):
File "/Users/me/dev/jsonargparse/jsonargparse/_typehints.py", line 840, in adapt_typehints
vals.append(adapt_typehints(val, subtype, **adapt_kwargs))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/me/dev/jsonargparse/jsonargparse/_typehints.py", line 789, in adapt_typehints
raise_unexpected_value(f"Expected a {typehint}", val)
File "/Users/me/dev/jsonargparse/jsonargparse/_typehints.py", line 711, in raise_unexpected_value
raise ValueError(message) from exception
ValueError: Expected a <class 'NoneType'>. Got value: Namespace(class_path='__main__.MySubModel', init_args=Namespace(loss='MSELoss'))
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/Users/me/dev/jsonargparse/regression.py", line 37, in <module>
cli = LightningCLI(MyBaseModel, subclass_mode_model=True)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/me/mambaforge/envs/jsonargparse/lib/python3.12/site-packages/lightning/pytorch/cli.py", line 394, in __init__
self.instantiate_classes()
File "/Users/me/mambaforge/envs/jsonargparse/lib/python3.12/site-packages/lightning/pytorch/cli.py", line 561, in instantiate_classes
self.config_init = self.parser.instantiate_classes(self.config)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/me/dev/jsonargparse/jsonargparse/_deprecated.py", line 147, in patched_instantiate_classes
cfg = self._unpatched_instantiate_classes(cfg, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/me/dev/jsonargparse/jsonargparse/_core.py", line 1275, in instantiate_classes
cfg[subcommand] = subparser.instantiate_classes(cfg[subcommand], instantiate_groups=instantiate_groups)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/me/dev/jsonargparse/jsonargparse/_deprecated.py", line 147, in patched_instantiate_classes
cfg = self._unpatched_instantiate_classes(cfg, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/me/dev/jsonargparse/jsonargparse/_core.py", line 1262, in instantiate_classes
parent[key] = component.instantiate_classes(value)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/me/dev/jsonargparse/jsonargparse/_typehints.py", line 618, in instantiate_classes
value[num] = adapt_typehints(
^^^^^^^^^^^^^^^^
File "/Users/me/dev/jsonargparse/jsonargparse/_typehints.py", line 848, in adapt_typehints
raise_union_unexpected_value(sorted_subtypes, val, vals)
File "/Users/me/dev/jsonargparse/jsonargparse/_typehints.py", line 718, in raise_union_unexpected_value
raise ValueError(
ValueError: Does not validate against any of the Union subtypes
Subtypes: [<class 'NoneType'>, <class '__main__.MyBaseModel'>]
Errors:
- Expected a <class 'NoneType'>
- Loss function MSELoss is not applicable for model MySubModel. Applicable loss functions are: ['BCEWithLogitsLoss', 'BCELoss', 'CosineEmbeddingLoss']
Given value type: <class 'jsonargparse._namespace.Namespace'>
Given value: Namespace(class_path='__main__.MySubModel', init_args=Namespace(loss='MSELoss'))Wrong config is printed for the subclass:
$ python regression.py fit --model MyBaseModel --print_config
...
model:
class_path: __main__.MyBaseModel
init_args:
loss: MSELoss
...
$ python regression.py fit --model MySubModel --print_config
...
model:
class_path: __main__.MySubModel
init_args:
loss: MSELoss # wrong default value
...Prior behavior
See description above.
Environment
- jsonargparse version: v4.39
- Python version: 3.12
- How jsonargparse was installed:
pip install jsonargparse[signatures] - OS: macOS 15.4.1
Metadata
Metadata
Assignees
Labels
bugSomething isn't workingSomething isn't working