Skip to content
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.

Sign up for GitHub

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 #1064

Merged
merged 2 commits into from
Aug 22, 2024
Merged

Remove NumPy <2 pin #1064

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
646da2f
Remove NumPy <2 pin.
seberg Aug 19, 2024
ec0fdcc
Include pre-release identifier for upper pin
seberg Aug 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dependencies.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ dependencies:
common:
- output_types: [conda, requirements, pyproject]
packages:
- numpy>=1.23,<2.0a0
- numpy>=1.23,<3.0a0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

  • conda-forge: 13.2.0 (link)
  • PyPI: 13.2.0 (link)

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. Minimal users (simply wrap GPU memory in CuPy arrays and do little else)
  2. 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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

- pynvml>=11.4.1
depends_on_cupy:
common:
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ authors = [
license = { text = "BSD-3-Clause" }
requires-python = ">=3.9"
dependencies = [
"numpy>=1.23,<2.0a0",
"numpy>=1.23,<3.0a0",
"pynvml>=11.4.1",
] # This list was generated by `rapids-dependency-file-generator`. To make changes, edit dependencies.yaml and run `rapids-dependency-file-generator`.
classifiers = [
Expand Down
Loading