Skip to content
This repository was archived by the owner on Sep 11, 2023. It is now read-only.

Remove PyTorch from the code #86

Closed
2 of 4 tasks
Tracked by #213
JackKelly opened this issue Sep 3, 2021 · 8 comments · Fixed by #215, #307 or #329
Closed
2 of 4 tasks
Tracked by #213

Remove PyTorch from the code #86

JackKelly opened this issue Sep 3, 2021 · 8 comments · Fixed by #215, #307 or #329
Assignees
Labels
enhancement New feature or request refactoring

Comments

@JackKelly
Copy link
Member

JackKelly commented Sep 3, 2021

When I started nowcasting_dataset, the intention was to use nowcasting_dataset to generate batches on-the-fly during ML training from separate Zarr stores for the satellite data, NWPs, and PV. But that turned out to be too slow and fragile :) So, we swapped to using nowcasting_dataset to pre-prepare batches ahead-of-time, and save them to disk. During ML training, we just need to load the batches from disk, and we're good-to-go. (Pre-preparing batches has a number of other advantages, too).

But, this development history means that nowcasting_dataset still uses PyTorch (e.g. using the PyTorch DataLoader to run multiple processes). The code may become cleaner and faster and more flexible if we strip out PyTorch, and instead (maybe) use concurrent.futures.ProcessPoolExecutor to use multiple processes.

TODO:

@peterdudfield
Copy link
Contributor

#213 (comment)
from big issue

Idea is to use optional requirements for pytorch

@JackKelly
Copy link
Member Author

If it's OK, I'll keep this issue open until we've removed the pytorch dataloader and pytorch lightning from the "batch pre-processing" code :)

@peterdudfield
Copy link
Contributor

sure thing, where is that?

@JackKelly
Copy link
Member Author

JackKelly commented Oct 11, 2021

Tthe specific places where pytorch / pytorch lightning are still used are:

I might strip out these PyTorch things in one of the sub-steps of #202 (but I haven't fully thought this through!)

@JackKelly JackKelly reopened this Oct 11, 2021
@JackKelly JackKelly mentioned this issue Oct 29, 2021
30 tasks
@JackKelly JackKelly linked a pull request Oct 29, 2021 that will close this issue
30 tasks
@peterdudfield
Copy link
Contributor

linked with - #315

@JackKelly JackKelly moved this to Todo in Nowcasting Nov 2, 2021
@JackKelly JackKelly added enhancement New feature or request refactoring labels Nov 2, 2021
Repository owner moved this from Todo to Done in Nowcasting Nov 2, 2021
@peterdudfield peterdudfield reopened this Nov 2, 2021
Repository owner moved this from Done to In Progress in Nowcasting Nov 2, 2021
@peterdudfield
Copy link
Contributor

peterdudfield commented Nov 2, 2021

Maybe its not quite closed, would be good to remove it from requirements too, ill have a go at this

@JackKelly
Copy link
Member Author

Oops, you're exactly right, sorry - this issue should still be open!

@JackKelly
Copy link
Member Author

JackKelly commented Nov 2, 2021

FWIW, these are the lines where "torch" is still mentioned in our code:
image

@peterdudfield peterdudfield mentioned this issue Nov 3, 2021
7 tasks
Repository owner moved this from In Progress to Done in Nowcasting Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request refactoring
Projects
No open projects
Status: Done
2 participants