Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lazy_instance no longer accepts arguments that go into kwargs #473

Closed
awaelchli opened this issue Mar 15, 2024 · 1 comment · Fixed by #476
Closed

lazy_instance no longer accepts arguments that go into kwargs #473

awaelchli opened this issue Mar 15, 2024 · 1 comment · Fixed by #476
Labels
bug Something isn't working

Comments

@awaelchli
Copy link

awaelchli commented Mar 15, 2024

🐛 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

  • jsonargparse version (e.g., 4.8.0): 4.27.6
  • Python version (e.g., 3.9): 3.10
  • How jsonargparse was installed (e.g. pip install jsonargparse[all]): pip install jsonargparse[signatures]
  • OS (e.g., Linux): Linux
@mauvilsa
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants