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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default config file fails to initialize module. #11622

Closed
ramonemiliani93 opened this issue Jan 25, 2022 · 5 comments 路 Fixed by #12061
Closed

Default config file fails to initialize module. #11622

ramonemiliani93 opened this issue Jan 25, 2022 · 5 comments 路 Fixed by #12061
Assignees
Labels
3rd party Related to a 3rd-party bug Something isn't working lightningcli pl.cli.LightningCLI
Milestone

Comments

@ramonemiliani93
Copy link
Contributor

ramonemiliani93 commented Jan 25, 2022

馃悰 Bug

Hi! Not sure if it is a misunderstanding on my side, when the default_config_files is used on the LightningCLI with a file containing the parameters for a LightningModule constructor it fails.

To Reproduce

main.py

from pytorch_lightning.utilities.cli import LightningCLI
from pl_bolts.models import LitMNIST


class MNIST(LitMNIST):
    def __init__(self, foo: int, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self.foo = foo


if __name__ == "__main__":
    cli = LightningCLI(MNIST, parser_kwargs={"default_config_files": ["default.yaml"]})

default.yaml

fit:
  model:
    foo: 123

To reproduce just put both on a folder and run python -m main fit inside.

Expected behavior

The fit routine should run with foo=123.

Environment

  • CUDA:
    - GPU:
    - available: False
    - version: None
  • Packages:
    - numpy: 1.22.1
    - pyTorch_debug: False
    - pyTorch_version: 1.10.1
    - pytorch-lightning: 1.5.9
    - tqdm: 4.62.3
  • System:
    - OS: Darwin
    - architecture:
    - 64bit
    -
    - processor: i386
    - python: 3.8.12
    - version: Darwin Kernel Version 21.2.0: Sun Nov 28 20:28:54 PST 2021; root:xnu-8019.61.5~1/RELEASE_X86_64

cc @carmocca @mauvilsa

@ramonemiliani93 ramonemiliani93 added the bug Something isn't working label Jan 25, 2022
@carmocca carmocca added the lightningcli pl.cli.LightningCLI label Jan 25, 2022
@carmocca
Copy link
Member

I can repro just with jsonargparse

class Foo:
    def __init__(self, foo: int):
        self.foo = foo

from jsonargparse import ArgumentParser, ActionConfigFile

parser = ArgumentParser(default_config_files=["default.yaml"])
parser.add_argument('--config', action=ActionConfigFile)
subcommands = parser.add_subcommands()

subparser = ArgumentParser()
subparser.add_class_arguments(Foo, nested_key="model")
subcommands.add_subcommand("fit", subparser)

subparser = ArgumentParser()
subparser.add_class_arguments(Foo, nested_key="model")
subcommands.add_subcommand("test", subparser)

args = parser.parse_args()
print(args)

Passing the config manually works as expected

$ python -m main --config=default.yaml fit
Namespace(config=[Path_fr(default.yaml, cwd=/Users/carlosmocholi/git/pytorch-lightning)], subcommand='fit', fit=Namespace(model=Namespace(foo=123)), __default_config__=Path_fr(default.yaml, cwd=/Users/carlosmocholi/git/pytorch-lightning))

But not this:

$ python -m main fit
usage: main.py [-h] [--config CONFIG] [--print_config [={comments,skip_null}+]] {fit,test} ...
main.py: error: Configuration check failed :: Key "fit.model.foo" is required but not included in config object or its value is None.

Can you verify that it is a bug upstream @mauvilsa?

@mauvilsa
Copy link
Contributor

Can you verify that it is a bug upstream @mauvilsa?

Yes, looks like a bug in jsonargparse.

@carmocca carmocca added the 3rd party Related to a 3rd-party label Jan 27, 2022
@mauvilsa
Copy link
Contributor

It seems that forgot to implement the use default_config_files from a parent parser in the nested subcommand parsers. In a sense this is more an entire feature missing than a simple bug fix.

@ramonemiliani93 for the time being, as a workaround define the default files for each subcommand. For example as LightningCLI(..., parser_kwargs={"fit": {"default_config_files": ["default_fit.yaml"]}}) and the default_fit.yaml would not have the fit: key.

@carmocca you can assign this issue to me.

@mauvilsa
Copy link
Contributor

The missing feature has been implemented in jsonargparse commit omni-us/jsonargparse@0d07c9a. This is included in version 4.3.0, which has just been released. Probably we shouldn't close this issue until a unit test is added to lightning to prevent regressions.

@fabian1heinrich
Copy link

It seems like the problem continues to occur, or at least a similar one.
I asked in the forum for help, but it seems more like a bug in my opinion.

https://lightning.ai/forums/t/problem-lightningcli-with-default-config-files/5963

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3rd party Related to a 3rd-party bug Something isn't working lightningcli pl.cli.LightningCLI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants