Easier configurability of callbacks that should always be present in LightningCLI

[mauvilsa]

What does this PR do?

Based on a discussion in slack it seemed convenient in LightningCLI to have callbacks that should always be present and be configurable. This only required a small change such that LightningArgumentParser.add_lightning_class_args also support callbacks and the callbacks added by this method be automatically added to the trainer. How to use it is explained in lightning_cli.rst.

Before submitting

• Was this discussed/approved via a GitHub issue? (not for typos and docs)
• Did you read the contributor guideline, Pull Request section?
• Did you make sure your PR does only one thing, instead of bundling different changes together?
• Did you make sure to update the documentation with your changes? (if necessary)
• Did you write any new necessary tests? (not for typos and docs)
• Did you verify new and existing tests pass locally with your changes?
• Did you update the CHANGELOG? (not for typos, docs, test updates, or internal minor changes/refactorings)

PR review

Anyone in the community is free to review the PR once the tests have passed.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:

• Is this pull request ready for review? (if not, please submit in draft mode)
• Check that all items from Before submitting are resolved
• Make sure the title is self-explanatory and the description concisely explains the PR
• Add labels and milestones (and optionally projects) to the PR so it can be classified

Did you have fun?

Make sure you had fun coding 🙃

[codecov]

Codecov Report

Merging #7964 (91cef79) into master (024cf23) will decrease coverage by 5%.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master   #7964    +/-   ##
=======================================
- Coverage      92%     87%    -5%     
=======================================
  Files         203     203            
  Lines       13151   13156     +5     
=======================================
- Hits        12042   11412   -630     
- Misses       1109    1744   +635     

[mauvilsa]

@jlperla

[awaelchli]

nice! this seems very useful

[carmocca]

Thanks a lot @mauvilsa

Can you also use this PR to mention that the config can be passed directly through CLI?

python script.py --config='{"trainer": {"callbacks": [{"class_path": "pytorch_lightning.callbacks.EarlyStopping", ...}]}}'

[ethanwharris]

Nice! Minor comment

[jlperla]

This is amazing!

The only thing that is missing as far as I can tell is showing how to change the default arguments. You often want to set a particular default that is different from the class constructors. For example, in your problem you may want to swap out the EarlyStopping to have a specific monitor variable and then let the user tweak the others. Or maybe change the default patience, etc.

[jlperla]

What about adding in a place to override default arugments here? It would make sense for code locality. Then something like

def add_lightning_class_args(
        self,
        lightning_class: Union[Type[Trainer], Type[LightningModule], Type[LightningDataModule]],
        nested_key: str,
        class_defaults: Dict[str, Any] = None,
        subclass_mode: bool = False
    ) -> None:
        """
        Adds arguments from a lightning class to a nested key of the parser
        Args:
            lightning_class: Any subclass of {Trainer, LightningModule, LightningDataModule, Callback}.
            nested_key: Name of the nested namespace to store arguments.
            subclass_mode: Whether allow any subclass of the given class.
            class_defaults: Set to override class defaults
        """

[mauvilsa]

I have added to the example in the docs an override of a default to illustrate how it could be done right now. Adding a class_defaults argument to add_lightning_class_args is a good idea. But this only makes sense if it works well with the subclass_mode option. Certainly can be done, but requires work and I can't do this today.