search

Lightning-Universe / lightning-flash

Your PyTorch AI Factory - Flash enables you to easily configure and run complex AI recipes for over 15 tasks across 7 data domains
https://lightning-flash.readthedocs.io
Apache License 2.0
1.74k stars 212 forks source link

Flash Zero: Allow to customize `LightningCLI` #1437

Open daavoo opened 2 years ago

daavoo commented 2 years ago

🚀 Feature

Expose options to customize the instantiation of LightningCLI in Flash Zero commands.

Motivation

I would like to be able to customize the instantiation. For example, to not use SaveConfigCallback which is hardcoded in:

https://github.com/Lightning-AI/lightning-flash/blob/0253d71588b582da326bb01ef116ebd7cd61f21e/flash/core/utilities/flash_cli.py#L166

Pitch

flash --save-config-callback False {task}

Alternatives

Do not use Flash Zero directly and instantiate LightningCLI manually.

krshrimali commented 2 years ago

Hi, @daavoo - Thank you for creating the issue! I think it makes sense, note that LightningCLI has save_config_callback and we can set it to None in LightningCLI but not in Flash Zero.

Myabe, we can think on the name, as having a boolean flag for save-config-callback might confuse users with save_config_callback in LightningCLI. Let's use, --save-config? cc: @ethanwharris - What do you think?

Just wondering, if you would like to take this up and create a PR? If you think you can help setup a draft PR, I can take it from there. :) Otherwise, I can help add this flag.

daavoo commented 2 years ago

Hi, @daavoo - Thank you for creating the issue! I think it makes sense, note that LightningCLI has save_config_callback and we can set it to None in LightningCLI but not in Flash Zero.

Yes, in the example I set it to False but it has the same effect as None because the check is if self.save_config_callback.

Myabe, we can think on the name, as having a boolean flag for save-config-callback might confuse users with save_config_callback in LightningCLI. Let's use, --save-config? cc: @ethanwharris - What do you think?

No strong opinion on name.

Just wondering, if you would like to take this up and create a PR? If you think you can help setup a draft PR, I can take it from there. :) Otherwise, I can help add this flag.

Sure thing. I wanted to open the issue first to check if it made sense for you

krshrimali commented 2 years ago

I think a PR to add a boolean flag --save-config to enable/disable save config callback will be a good idea! Regarding the name, if there are any other suggestions, we can rename it quickly in the PR before merging. :)

I'll be looking forward to your PR, thank you very much!

Borda commented 1 year ago

@daavoo are you interested in looking at it?