-
Notifications
You must be signed in to change notification settings - Fork 3.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Drop the numpy requirement #16649
Comments
I think there was some cross-dependency, leaking |
We had a strong dependency on numpy for working with jagged arrays when post-processing the various output formats for the epoch_end hooks. |
There's this issue on PyTorch to include the SeedSequence API: pytorch/pytorch#56805, but as a workaround, they added pytorch/pytorch#56797 This PR can be done in pieces, we don't need to replace all usages concurrently. |
@carmocca , if this issue is planned to be done in pieces, I can help with some module. let me know. |
@dingusagar You can go ahead and replace the usages you are confident about. |
This is the end goal. Some of our numpy usages are easy to replace, others not so much. You can start doing the easy replacements first, get those merged, and then addressing the hard ones separately after. Once all are done, the requirement can be removed. |
Okay, I will get started with it. @carmocca can you assign this issue to me? |
I have removed the numpy usage for the callback scripts, and raised a PR #17267 |
I'd also like to help on this. @ishandutta0098 I can work on the model summary if you haven't yet. |
Hi @ryan597 I have made changes to most of the files in lightning and they are currently on my local branch, the testing was pending so I didn't create a pull request for them yet. However I haven't changed anything in the examples folder yet, maybe you can give it a try. If you do kindly share the files you are modifying so that we don't change the same ones! |
Sure thanks. I did a quick grep through the files and see numpy in these examples. They look like quick changes so I can do these. examples/fabric/meta_learning/train_fabric.py Edit to include: |
Okay, I would skip these files then. |
Hey are you guys still working on this issue? Anything I could contribute? |
You can take it =) |
Hi @Peiffap, which files in |
Hi @Bhavay-2001, cheers for checking in with me on this!
I'm not currently actively working on this issue, but I think it would make sense in general to create a task list for this issue with the work left to do (i.e. the files or directories with numpy dependencies), in order to keep track of who's doing what/which tasks remain.
I'm not sure I understand what you mean here. |
Hi @Peiffap, I will make a list of files that need to be changed or updated from numpy to torch and will share. Then I can work on my half of files and you can work on yours. |
That sounds good!
I don't currently plan on working on this, but in any case it'd be good to have a place for people wanting to contribute to find which files are being worked on by someone else already. |
There are mainly 3-4 files in the |
Hi
Thanks everyone for the willingness to contribute. I hope the directions above make it a bit clearer what should be done as the next step. |
So, the goal is to make the numpy an optional dependency ? Thank you for explaining it a bit more, I'll pick one of the issues you mentioned. |
Outline & Motivation
We barely use
numpy
. We should replace its uses with torch.Reduce the package footprint
Pitch
Remove https://github.com/Lightning-AI/lightning/blob/7bbbe22636853e1ee6fa57b557b3408d45233589/requirements/pytorch/base.txt#L4
Update numpy usages with torch.
Additional context
No response
cc @Borda @justusschock @awaelchli
The text was updated successfully, but these errors were encountered: