-
Notifications
You must be signed in to change notification settings - Fork 38
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
Remove NumPy <2 pin #208
Remove NumPy <2 pin #208
Conversation
This project has a very light NumPy and no CuPy dependency, so pinning was briefly useful for the transition when NumPy 2 came out, but unnecessary as soon as there were no obvious Python incompatibilities. |
@jakirkham I am a bit confused by the result here. The tests seem to pick up NumPy 2.1 (good), but are also forced to use pytorch 2.0.0. However, Considering how simple the dependency on NumPy is, I don't have a concern at all here (that is a torch/NumPy problem), but it does confuse me right now. EDIT: To clarify this confusion. pytorch seems installed in a pretty aggressive way, bypassing the incompatible versions. But |
Realized that |
5507030
to
2a33517
Compare
Sebastian and I discussed the PyTorch question offline We observed a few things about PyTorch usage here:
These are documented in issue: #210 This led to a separate question. PyTorch only started supporting NumPy 2 in PyTorch 2.3.0 ( pytorch/pytorch#107302 (comment) ). However the versions here are older than that. We observed:
While it doesn't appear necessary to change PyTorch here, we still think it is a good idea to add the PyTorch dependency and update the versions used. However we don't know enough about how PyTorch is used here to develop a solution without more discussion. To facilitate discussion and reaching an acceptable solution, have raised issue: #210 |
/merge |
44e8a4c
into
rapidsai:branch-24.10
Thanks Sebastian! 🙏 |
This PR removes the NumPy<2 pin.
wholegraph
does not appear to be a heavy user of NumPy or CuPy, so it should be fine to simply remove the pin.For other RAPIDS projects with heavier dependency, CuPy 13.3.0 was required (just released) to have sufficient good CuPy/NumPy interoperability.