lightning-flash icon indicating copy to clipboard operation
lightning-flash copied to clipboard

Integrate `lightning_utilities`

Open carmocca opened this issue 3 years ago • 10 comments

🚀 Feature

We have released https://github.com/Lightning-AI/utilities/ which contains multiple utilities that are shared across the codebase.

Motivation

Avoid protected imports Avoid duplicated code

Pitch

Go through the Flash codebase and see which duplications or protected imports we have

carmocca avatar Sep 06 '22 12:09 carmocca

Examples:

https://github.com/Lightning-AI/lightning-flash/blob/674232381ad9e984a256037749fa714fcfd1b627/flash/core/utilities/imports.py#L31-L75

https://github.com/Lightning-AI/lightning-flash/blob/674232381ad9e984a256037749fa714fcfd1b627/flash/core/utilities/apply_func.py#L34-L41

There should be DeprecationWarnings for utilities imported from PL that should start using lightning_utilities

carmocca avatar Sep 06 '22 12:09 carmocca

Hi, @carmocca - Thank you! I am up for it, and it's only going to help us stay stable with respect to these utilities.

Is it possible that we can create a list of the sections of code that we can change, and take some help from our awesome community?

krshrimali avatar Sep 08 '22 13:09 krshrimali

The two I pointed out in my previous message are the only ones I could find. These are the ones we currently have in the utilities: https://github.com/Lightning-AI/utilities/tree/main/src/lightning_utilities/core

Should be a very easy PR, up for grabs!

carmocca avatar Sep 08 '22 15:09 carmocca

The two I pointed out in my previous message are the only ones I could find. These are the ones we currently have in the utilities: https://github.com/Lightning-AI/utilities/tree/main/src/lightning_utilities/core

Should be a very easy PR, up for grabs!

Thanks! @uakarsh - Maybe you'll like to pick these up? Please let us know 🎉

krshrimali avatar Sep 08 '22 15:09 krshrimali

For examples of the PR in lightning: https://github.com/Lightning-AI/lightning/pull/14475, and https://github.com/Lightning-AI/lightning/pull/14537

carmocca avatar Sep 08 '22 15:09 carmocca

Hi @carmocca @krshrimali, this definitely looks interesting. Can you give me some time, to explore about lightning_utility, and the existing codebase? I would surely get back to you.

uakarsh avatar Sep 08 '22 15:09 uakarsh

Hi @carmocca @krshrimali, this definitely looks interesting. Can you give me some time, to explore about lightning_utility, and the existing codebase? I would surely get back to you.

Oh definitely, please take your time and let us know if you need any help! Thank you for your help! ❤️

krshrimali avatar Sep 08 '22 15:09 krshrimali

HI, @krshrimali @carmocca, I would now start working on this task. So, far I have been going through the lightning_utilities repo, and what I could understand is that the repo's main idea is to centralize the steps involved before starting the code, i.e module availability, the checking the data structures required for different methods, and converting them to some other data structure. Although there were many other steps involved, these were something, I was able to understand.

So, my current plan is to g through the entire codebase of lightning flash, and replace all the methods that come under lightning_utilities with them.

Anything, which I missed or should be included while modifying the codebase?

uakarsh avatar Sep 15 '22 13:09 uakarsh

Nothing missed. That's perfect :)

carmocca avatar Sep 15 '22 14:09 carmocca

Cool, would be starting today

uakarsh avatar Sep 15 '22 14:09 uakarsh