Skip to content

Improve LightningCLI #15115

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

Closed
gianmarcoaversanoenx opened this issue Oct 13, 2022 · 9 comments · Fixed by omni-us/jsonargparse#182
Closed

Improve LightningCLI #15115

gianmarcoaversanoenx opened this issue Oct 13, 2022 · 9 comments · Fixed by omni-us/jsonargparse#182
Labels
lightningcli pl.cli.LightningCLI pl Generic label for PyTorch Lightning package question Further information is requested won't fix This will not be worked on

Comments

@gianmarcoaversanoenx
Copy link

gianmarcoaversanoenx commented Oct 13, 2022

🚀 Feature

Allow declaration of any object in LightningCLI, similarly to Hydra.

Motivation

LightningCLI can't be used for complex cases as is.

Pitch

Setting up the list of transforms won't work.

data:
  class_path: brainiac_2.datasets.CIFARDataModule
  init_args:
    data_dir: .data
    batch_size: 32
    num_workers: 1
    cifar_n: 10
    split:
    - 45000
    - 5000
    transforms:
      class_path: torchvision.transforms.Compose
      init_args:
        transforms:
          - class_path: torchvision.transforms.Resize
            init_args:
              size: 64
          - class_path: torchvision.transforms.ToTensor
            init_args:
          - class_path: torchvision.transforms.Normalize
            init_args:
              mean: [0.5, 0.5, 0.5]
              std: [0.5, 0.5, 0.5]

Another examples with callbacks

trainer:
  logger: true
  enable_checkpointing: true
  callbacks:
    - class_path: pytorch_lightning.callbacks.EarlyStopping
      init_args:
        monitor: 'loss/val'
        min_delta: 0. # minimum change in the monitored quantity
        patience: 3 # checks with no improvement after which training will be stopped
        verbose: False # verbosity mode
        mode: "min"
        strict: True # crash the training if monitor is not found
        check_finite: True # stops training when the monitor becomes NaN or infinite
    - class_path: brainiac_2.mace.MaceCallback
      init_args:
        membership_prior: 0.5
        use_kde: True
        query:
          class_path: brainiac_2.query.QueryDiscriminator
          init_args: null

I need lists of non-custom objects, which requires other non-custom objects as input arguments, but no way to achieve it.

Alternatives

If this is already possible, then there should be an example in the doc, which only covers the most basic cases.

cc @carmocca @mauvilsa


If you enjoy Lightning, check out our other projects! ⚡

  • Metrics: Machine learning metrics for distributed, scalable PyTorch applications.

  • Lite: enables pure PyTorch users to scale their existing code on any kind of device while retaining full control over their own loops and optimization logic.

  • Flash: The fastest way to get a Lightning baseline! A collection of tasks for fast prototyping, baselining, fine-tuning, and solving problems with deep learning.

  • Bolts: Pretrained SOTA Deep Learning models, callbacks, and more for research and production with PyTorch Lightning and PyTorch.

  • Lightning Transformers: Flexible interface for high-performance research using SOTA Transformers leveraging PyTorch Lightning, Transformers, and Hydra.

@gianmarcoaversanoenx gianmarcoaversanoenx added the needs triage Waiting to be triaged by maintainers label Oct 13, 2022
@carmocca carmocca added question Further information is requested lightningcli pl.cli.LightningCLI pl Generic label for PyTorch Lightning package and removed needs triage Waiting to be triaged by maintainers labels Oct 13, 2022
@mauvilsa
Copy link
Contributor

I am investigating what to do about this.

@mauvilsa
Copy link
Contributor

@gianmarcoaversanoenx the second example with brainiac_2.mace.MaceCallback is that your code or some project you contribute to? If so, you should add accurate type hints to the code. This way LightningCLI will instantiate as you want and do a better job validating and giving error messages. If you are unsure about how to add the types, please ask.

For the torchvision example, again the proper fix is that they add type hints to the transforms. I checked to see what is the state of that, see pytorch/vision#2025 comment. Seems there is still some time to go until this happens.

Since for projects like torchvision the types take time, I created a pull request to support classes when the type is Any, jsonargparse#182. When the code does not have types, it defaults to Any, so the examples above would work with this. @gianmarcoaversanoenx since you asked for this feature, can you please your examples with that branch and also review the pull request?

Do note that supporting instantiation with type Any is not a replacement to adding accurate type hints. Only with accurate types the full set of features become available.

@gianmarcoaversanoenx
Copy link
Author

gianmarcoaversanoenx commented Oct 24, 2022

Hello
thanks a lot!

I still have a question: I do have type hints in place for everything, but why are they so necessary? I mean, with Hydra, one can simply write whatever and it works out of the box. Example:

trainer:
  callbacks:
    - _target_: my_package.my_module.CoolCallback
      arg1: val1
      arg2: val2
      arg3:
         _target_: torchvision.transform.Compose
       # etc

and Hydra will simply try to import whatever is specified in the _target_ field. The torchvision part never gave me any problem with Hydra. I'm struggling with LightningCli only.

@mauvilsa
Copy link
Contributor

A purpose of the parser is that when given wrong input it fails as early as possible with a useful message. And the parser uses the type hints to know how to validate the input. Imagine that you add the wrong value in a config, str instead of int, this value is used after the first epoch ends, and an epoch takes hours. If the code does have a type hint then the CLI will fail after a couple of seconds saying that an int was expected. Without a type hint it will fail after hours and most likely with a not easy to understand stack trace.

@mauvilsa
Copy link
Contributor

@gianmarcoaversanoenx will you test and review the pull request?

@gianmarcoaversanoenx
Copy link
Author

Alright! I'll check it out.

@gianmarcoaversanoenx
Copy link
Author

@mauvilsa

I git-cloned the branch and tried to run my script and all good. See: omni-us/jsonargparse#182

@mauvilsa
Copy link
Contributor

mauvilsa commented Oct 28, 2022

@gianmarcoaversanoenx this issue can be closed. There is no need to change anything in pytorch-lightning. Whoever needs this can just update to the latest version of jsonargparse.

@stale
Copy link

stale bot commented Apr 14, 2023

This issue has been automatically marked as stale because it hasn't had any recent activity. This issue will be closed in 7 days if no further activity occurs. Thank you for your contributions - the Lightning Team!

@stale stale bot added the won't fix This will not be worked on label Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lightningcli pl.cli.LightningCLI pl Generic label for PyTorch Lightning package question Further information is requested won't fix This will not be worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants