CLI: add arg - instantiate_only

[Borda]

What does this PR do?

allow instantiate_only

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 #8251 (14b5a6c) into master (365a9ba) will increase coverage by 4%.
The diff coverage is 50%.

@@           Coverage Diff           @@
##           master   #8251    +/-   ##
=======================================
+ Coverage      88%     92%    +4%     
=======================================
  Files         212     213     +1     
  Lines       13701   13800    +99     
=======================================
+ Hits        12125   12732   +607     
+ Misses       1576    1068   -508     

[carmocca]

Can you elaborate why one would want this?

[Borda]

Can you elaborate why one would want this?

at him moment you have to always run the fit, right? but if you want just to get the instances of Trainer, Model, and Data and then your own staff you need to overwrite CLI.fit(...) to be empty...
moreover (well it is docs) but from API is it not intuitive that with creating a CLI instance it automatically perform training

[carmocca]

at him moment you have to always run the fit, right? but if you want just to get the instances of Trainer, Model, and Data and then your own staff you need to overwrite CLI.fit(...) to be empty...
moreover (well it is docs) but from API is it not intuitive that with creating a CLI instance it automatically perform training

This is the whole point of the CLI. With #7508 we could make it so not passing the function to run has the same effect as what this PR wants.

But this flag doesn't fit right with me when the only purpose of the CLI is to fit right now.

cc @mauvilsa for thoughts

[Borda]

But this flag doesn't fit right with me when the only purpose of the CLI is to fit right now.

for me, the main purpose of CLI is to instantiate all main actors - Trainer, Model and Data
then the fix is something extra and so I would rather have it as optional than mandatory

So what I have to do for use Tuner:

class TuneFitCLI(LightningCLI):

    def before_fit(self) -> None:
        """Implement to run some code before fit is started"""
        res = self.trainer.tune(**self.fit_kwargs, scale_batch_size_kwargs=dict(max_trials=5))
        self.instantiate_classes()
        torch.cuda.empty_cache()
        self.datamodule.batch_size = int(res['scale_batch_size'] * 0.9)


if __name__ == '__main__':
    cli = TuneFitCLI(
        model_class=MultiPlantPathology,
        datamodule_class=PlantPathologyDM,
        trainer_defaults=TRAINER_DEFAULTS,
        seed_everything_default=42,
    )

[mauvilsa]

I think we should not add a skip_fit parameter. The purpose of LightningCLI is to run a command. As @carmocca said, right now it is fit, but with #7508 there will be multiple commands. Having this parameter would not make much sense once #7508 is done.

[Borda]

I think we should not add a skip_fit parameter. The purpose of LightningCLI is to run a command. As @carmocca said, right now it is fit, but with #7508 there will be multiple commands. Having this parameter would not make much sense once #7508 is done.

So you say that if don't want to run any Trainer method by default I cannot use CLI or need to overwrite the CLI.fit()?
@PyTorchLightning/core-contributors

[mauvilsa]

I think we should not add a skip_fit parameter. The purpose of LightningCLI is to run a command. As @carmocca said, right now it is fit, but with #7508 there will be multiple commands. Having this parameter would not make much sense once #7508 is done.

So you say that if don't want to run any Trainer method by default I cannot use CLI or need to overwrite the CLI.fit()?
@PyTorchLightning/core-contributors

I am saying that skip_fit parameter conflicts with #7508. If there is a need that LightningCLI not run on instantiation, then there would be a run disable, not specific to fit. Anyway you always want to run something. The idea is that this something is implemented as part of a subclass of LightningCLI. Maybe can you explain better why use LightningCLI without running anything?

[Borda]

Maybe can you explain better why use LightningCLI without running anything?

for example for the tune, or does extra action before fit or just print model details, you can see any past CLI in Bolts

@mauvilsa so are we just about the name? so let's call it intantiate_only?

[tchaton]

Hey @mauvilsa @carmocca @Borda,

I understand Jirka use-case. I believe more users might want to rely on the LightningCLI only as an instantiation tool.
I believe having an instantiate_only=False argument might be useful in the long run.

Best,
T.C

[carmocca]

The discussion is scattered on both PR. Writing here what I think is the best way forward is:

• This flag is not added as it would be removed with Call any trainer function from the LightningCLI #7508
• We add support for subcommand as proposed in Call any trainer function from the LightningCLI #7508
• Call any trainer function from the LightningCLI #7508 allows for not passing any subcommand which means we just parse as this PR intends. A warning will be printed in this case as it's not a common use-case.
• In a follow-up PR, we add support for (some) combinations of common subcommands, for example: fit-and-test, and tune-and-fit.

[Borda]

@carmocca if your PR has the option to not run any trainer method, then it is fine and we do not need this :]

[tchaton]

@Borda @carmocca Good to close this PR then ?

[Borda]

closing this one in favor of #7508