omni-us / jsonargparse

Implement minimal boilerplate CLIs derived from type hints and parse from command line, config files and environment variables
https://jsonargparse.readthedocs.io
MIT License
314 stars 42 forks source link

`lazy_instance` no longer accepts arguments that go into kwargs #473

Closed awaelchli closed 5 months ago

awaelchli commented 5 months ago

🐛 Bug report

The last release of jsonargparse 4.27.6 made our CI fail. We have a test that does the following:

To reproduce

pip install lightning
import torch
from jsonargparse import lazy_instance

from lightning.pytorch import LightningModule
from lightning.pytorch.cli import LightningCLI
from lightning.pytorch.strategies import DDPStrategy

class BoringModel(LightningModule):
    def __init__(self):
        super().__init__()
        self.layer = torch.nn.Linear(32, 2)

    def training_step(self, batch, batch_idx):
        loss = self.layer(batch).sum()
        self.log("train_loss", loss)
        return {"loss": loss}

    def configure_optimizers(self):
        return torch.optim.SGD(self.layer.parameters(), lr=0.1)

def run():
    strategy_default = lazy_instance(DDPStrategy, find_unused_parameters=True)
    LightningCLI(
        BoringModel,
        run=False,
        trainer_defaults={"strategy": strategy_default},
    )

if __name__ == "__main__":
    run()

Error:

Traceback (most recent call last):
  File "/Users/adrian/miniconda3/envs/lightning/lib/python3.11/site-packages/jsonargparse/_core.py", line 1083, in check_config
    check_values(cfg)
  File "/Users/adrian/miniconda3/envs/lightning/lib/python3.11/site-packages/jsonargparse/_core.py", line 1077, in check_values
    raise NSKeyError(f'No action for key "{key}" to check its value.')
jsonargparse._namespace.NSKeyError: No action for key "find_unused_parameters" to check its value.

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/adrian/miniconda3/envs/lightning/lib/python3.11/site-packages/jsonargparse/_core.py", line 441, in parse_object
    parsed_cfg = self._parse_common(
                 ^^^^^^^^^^^^^^^^^^^
  File "/Users/adrian/miniconda3/envs/lightning/lib/python3.11/site-packages/jsonargparse/_core.py", line 319, in _parse_common
    self.check_config(cfg, skip_required=skip_required)
  File "/Users/adrian/miniconda3/envs/lightning/lib/python3.11/site-packages/jsonargparse/_core.py", line 1089, in check_config
    raise type(ex)(message) from ex
jsonargparse._namespace.NSKeyError: Validation failed: No action for key "find_unused_parameters" to check its value.

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/adrian/miniconda3/envs/lightning/lib/python3.11/site-packages/jsonargparse/_typehints.py", line 1277, in check_lazy_kwargs
    parser.parse_object(lazy_kwargs)
  File "/Users/adrian/miniconda3/envs/lightning/lib/python3.11/site-packages/jsonargparse/_deprecated.py", line 124, in patched_parse
    cfg = parse_method(*args, _skip_check=_skip_check, **kwargs)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/adrian/miniconda3/envs/lightning/lib/python3.11/site-packages/jsonargparse/_core.py", line 451, in parse_object
    self.error(str(ex), ex)
  File "/Users/adrian/miniconda3/envs/lightning/lib/python3.11/site-packages/jsonargparse/_core.py", line 1002, in error
    raise argument_error(message) from ex
argparse.ArgumentError: Validation failed: No action for key "find_unused_parameters" to check its value.

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/adrian/repositories/lightning/examples/pytorch/bug_report/bug_report_model.py", line 33, in <module>
    run()
  File "/Users/adrian/repositories/lightning/examples/pytorch/bug_report/bug_report_model.py", line 24, in run
    strategy_default = lazy_instance(DDPStrategy, find_unused_parameters=True)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/adrian/miniconda3/envs/lightning/lib/python3.11/site-packages/jsonargparse/_typehints.py", line 1349, in lazy_instance
    return lazy_init_class(class_type, kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/adrian/miniconda3/envs/lightning/lib/python3.11/site-packages/jsonargparse/_typehints.py", line 1285, in __init__
    check_lazy_kwargs(class_type, lazy_kwargs)
  File "/Users/adrian/miniconda3/envs/lightning/lib/python3.11/site-packages/jsonargparse/_typehints.py", line 1279, in check_lazy_kwargs
    raise ValueError(str(ex)) from ex
ValueError: Validation failed: No action for key "find_unused_parameters" to check its value.

Expected behavior

I'm not sure, maybe it's intentionally no longer supported, but it worked in 4.27.5. The find_unused_parameters in the above usage is not explicitly listed in the DDPStrategy signature, but is supposed to be eaten by the **kwargs: https://github.com/Lightning-AI/pytorch-lightning/blob/5232cfdc77df1df2fa712c4ec7781266ae572daa/src/lightning/pytorch/strategies/ddp.py#L85

Do you suggest I change the test in Lightning?

Environment

mauvilsa commented 5 months ago

That was unintended, sorry! I will fix as soon as possible.