Add LightningCLI(run=False|True)

[carmocca]

What does this PR do?

Fixes #8615, #8251
Part of #7508

After discussing with @mauvilsa, we came to the conclusion that the solution of having a __init__(run: bool) flag to disable fit is better than doing it by not passing a subcommand:

# what this PR implements
# LightningCLI(..., run=False)
$ python script.py ...

# vs

# what was the previous idea
# LightningCLI(...)
$ python script.py ...

This is because there is no case where the user would go from doing python script.py fit <some_args> to python script.py <some_args> without code changes because to do so, the user would need to write cli.trainer.fit(cli.model) anyways after the CLI instantiation.

With that in consideration, we require the user to explicitly set run=False.
This way, you have to really mess up to run fit twice accidentally.

Does your PR introduce any breaking changes? If yes, please list them.

None

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 list all the breaking changes introduced by this pull request?
• Did you update the CHANGELOG? (not for typos, docs, test updates, or internal minor changes/refactorings)

PR review

• 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

[codecov]

Codecov Report

Merging #8751 (ad6458d) into master (f1cc6e3) will decrease coverage by 4%.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master   #8751    +/-   ##
=======================================
- Coverage      93%     89%    -4%     
=======================================
  Files         169     169            
  Lines       14068   14069     +1     
=======================================
- Hits        13038   12461   -577     
- Misses       1030    1608   +578     

[Borda]

looks the same as mine #8251 which you blocked :P

[mauvilsa]

Since there will be multiple ways of using LightningCLI there is one thing that could be done to prevent mistakes. When run==True after running, the Trainer instance could be modified overriding the fit, test, etc. methods such that it shows an error instead of running.

[carmocca]

after running, the Trainer instance could be modified overriding the fit, test, etc. methods such that it shows an error instead of running.

This would limit those who want to fit with run=True and still call test manually later (or fit again!)

[tchaton]

Hey @carmocca.

Can you share why run is set to True by default ?
This seems counter-intuitive as you could argue that test should also be run too.
Furthermore, it doesn't fully resolve #8615 as it will run fit by default.
IMO, I would prefer user to opt-in on a running fit to avoid confusion.

[carmocca]

Can you share why run is set to True by default ?

Because the entire design is focused on getting up-and-running as easily as possible.

Setting it to False would require a full restructure of the docs.

Furthermore, it doesn't fully resolve #8615 as it will run fit by default.

We can remove that issue from the "fixes" list. This PR adds support for having the choice

This seems counter-intuitive

The CLI docs only mention that it is designed to run fit by default so that is documented.

you could argue that test should also be run too.

This is entirely fair, running fit only was just the initial design. #7508 will add support for test

I would prefer user to opt-in on a running fit to avoid confusion.

Again, #7508 will make it opt-in as providing a subcommand will be a requirement with run=True (default)