Remove NumPy <2 pin #1064
Remove NumPy <2 pin #1064
Conversation
| @@ -136,7 +136,7 @@ dependencies: | |||
| common: | |||
| - output_types: [conda, requirements, pyproject] | |||
| packages: | |||
| - numpy>=1.23,<2.0a0 | |||
| - numpy>=1.23,<3.0a0 | |||
There was a problem hiding this comment.
Putting this in a comment on the diff so we can thread the conversation.
The description of this PR says that it'll be safe to remove this pin "once CuPy 13.3.0 is released".
It hasn't been yet.
Could you share the relevant context on the issues with using CuPy 13.2.0 and NumPy 2.x? I don't see it at rapidsai/build-planning#38.
There was a problem hiding this comment.
To provide some clarity, CuPy 13.2.0 shipped with NumPy 2 support
However there were a couple of bugs that we wanted to fix upstream. Most notably these two:
We switched to a project board for tracking. Unfortunately that board is private (as that is the default visibility). There is nothing private there. However you can find it under RAPIDS Projects where it is called "NumPy 2 Bringup"
Sebastian, myself and few others have been working with upstream to fix these bugs and test them using packages we have been building here: conda-forge/cupy-feedstock#272
On the testing side, the changes in CuPy's v13 are looking good
When talking with PfN offline earlier this week, they stated that we should be good to wrap things up and release Thursday (they are based in Japan so that may very well be Wednesday for some). They have generated some artifacts already and are just finalizing some things (guessing QA) before publishing
With the release on route, Sebastian and I decided to go ahead and get these up and start the review process
These PRs were generated with rapids-reviser so contain the same messaging in every case. That said, it is worth noting there are two kinds of CuPy consumers in RAPIDS:
- Minimal users (simply wrap GPU memory in CuPy arrays and do little else)
- General users (perform more operations on CuPy arrays and may write algorithms with them)
Sebastian and I decided that it was safe to open ready to review PRs for minimal users and leave PRs in draft for general users. UCX-Py is in the minimal user category
Hope that helps. Please feel free to ask more questions as needed
There was a problem hiding this comment.
Got it! Ok thanks very much for that.
I do think as a general rule, the fact that rapids-reviser or any other automation was used isn't a strong justification for the PR description being inaccurate. Especially since in RAPIDS repos, the PR description is added automatically to the commit log when you ask the bot to merge. Example: b0cbd6e
I think it'd be good for you or @seberg to modify this and the other "minimal users" PR descriptions before any of them are merged.
But that's enough context for me to review this and the other "minimal users". Based on what's currently open from the list at rapidsai/build-planning#38 (comment) and what I see on the NumPy 2 Bringup board, I guess that's just these:
There was a problem hiding this comment.
Yeah think that is a good point. Thanks James! 🙏
Yep that sounds right. The IO libraries think of containers of memory on the CPU or the GPU. Not doing math or other operations, which are left to other projects
There was a problem hiding this comment.
Makes sense to me, based on @jakirkham 's description in #1064 (comment).
I spot-checked a few logs and it looks like NumPy 2.1.0 was getting pulled in. I don't see any other issues.
Please do update the PR description (removing the inaccurate language about CuPy 13.3.0) before merging.
|
You are right, the description was not good! CuPy 13.3 is pretty meaningless for these very light users of both APIs (as John detailed). Basically, the only reason for not removing the pin long ago was that the rapids project have/had all the identical pins. I updated the description for the "ready" PRs (not the commit messages, but I think the merge includes the PR description). |
|
/merge |
|
Thanks Sebastian and James! 🙏 |
|
Great, thanks very much @seberg ! I appreciate you both moving this effort forward, I know it's a lot of work. |
This PR removes the NumPy<2 pin. This is desirable for RAPIDS projects that do not have a strong numpy/cupy dependency.
Not doing any math, they don't run into subtle issues, but the pin was still necessary briefly before NumPy 2 was initially released.
(Other RAPIDS projects rely heavily on CuPy/NumPy and need the soon to be released 13.3.0 to function properly, e.g. due to promotion related changes in NumPy.)