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.

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

Update numpy version specifier #480

Merged
merged 3 commits into from
Jan 18, 2023

Conversation

ajschmidt8
Copy link
Member

This PR updates the numpy version specifier for cucim.

It was previously updated in this commit to enable publishing Python 3.10 packages.

However, since there was no maximum version in that specifier, conda was picking up the latest version of numpy (1.24.1) which is incompatible with cudf's version of numba (numba>=0.56.2).

For instance, if you try to run this: mamba create -n test 'numba>=0.56.2' 'numpy>=1.24.1', it will fail to solve.

Running with the new version specifier in this PR does solve: mamba create -n test 'numba>=0.56.2' 'numpy>=1.21.3,<1.24'.

Therefore, this maximum version limit seems necessary for cucim to be compatible with the rest of RAPIDS.

@ajschmidt8 ajschmidt8 added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Jan 16, 2023
@ajschmidt8 ajschmidt8 requested a review from a team as a code owner January 16, 2023 19:54
@ajschmidt8
Copy link
Member Author

This PR should unblock rapidsai/dask-cuda#1080.

@bdice
Copy link
Contributor

bdice commented Jan 16, 2023

I'm not sure if this is the right fix here. Only packages using numba should have a upper bound on numpy, and it doesn't look like this package depends on numba. Also, this upper bound pinning only applies until numba 0.57.0 is released (which adds support for newer numpy versions).

Instead, shouldn't we be pinning only cudf (and not cucim)? https://github.com/rapidsai/cudf/blob/36b07f13480d8f7ba18274cb3a63f06855d848ff/conda/recipes/cudf/meta.yaml#L65

@@ -43,7 +43,7 @@ requirements:
- cudatoolkit ={{ cuda_version }}
- cupy >=10,<12.0.0a0
- libcucim ={{ version }}
- numpy >=1.21.3
- numpy >=1.21.3,<1.24
Copy link
Contributor

Choose a reason for hiding this comment

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

Discussed with @ajschmidt8 offline. This was solving to 1.24.1 in the host, which meant that the runtime pin_compatible was something like >=1.24 -- which is incompatible with cudf.

The solution is that host requirements should build against the oldest supported NumPy. Then the pin_compatible won't conflict with cudf's requirements.

Suggested change
- numpy >=1.21.3,<1.24
- numpy =1.21.3

@codecov-commenter
Copy link

codecov-commenter commented Jan 16, 2023

Codecov Report

Base: 92.95% // Head: 92.95% // No change to project coverage 👍

Coverage data is based on head (5dd1f48) compared to base (c899ea2).
Patch has no changes to coverable lines.

Additional details and impacted files
@@              Coverage Diff              @@
##           branch-23.02     #480   +/-   ##
=============================================
  Coverage         92.95%   92.95%           
=============================================
  Files               130      130           
  Lines              9775     9775           
=============================================
  Hits               9086     9086           
  Misses              689      689           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ajschmidt8
Copy link
Member Author

/merge

@rapids-bot rapids-bot bot merged commit 2acc027 into rapidsai:branch-23.02 Jan 18, 2023
@ajschmidt8 ajschmidt8 deleted the numpy-pin branch January 18, 2023 15:10
@jakirkham
Copy link
Member

Thanks all! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improves an existing functionality non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants