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

Unpin numpy in Dask environment, leave dask pinned in Dask-SQL environment #88

Merged
merged 2 commits into from
Mar 14, 2024

Conversation

charlesbluca
Copy link
Member

@charlesbluca charlesbluca commented Mar 14, 2024

Unblocks remaining image build failures after the merging of dask-contrib/dask-sql#1314:

@jakirkham
Copy link
Member

Yeah we needed to bump to NumPy 1.23 as part of supporting Python 3.11 ( rapidsai/build-planning#3 (comment) )

Should add recently we added an upper bound to NumPy in preparation for NumPy 2 ( rapidsai/build-planning#29 ). We plan to look into relaxing this, but need time to test (and potentially fix issues that come up). So the pin gives us breathing room

@jakirkham
Copy link
Member

Also looking upstream it appears Dask uses NumPy 1.22 with Python 3.9 tests. Idk if that is an issue for us, so mentioning just in case

Other environments seem to use NumPy 1.23 or newer (some are unconstrained)

@charlesbluca
Copy link
Member Author

Also looking upstream it appears Dask uses NumPy 1.22 with Python 3.9 tests. Idk if that is an issue for us, so mentioning just in case

Yup that's essentially what this PR is aiming to work around by manually unpinning in the Dockerfile - typically these unpins are aimed at the 3.9 (or lowest minor Python version) environment where some of the core dependencies are often pinned in ways that are incompatible with RAPIDS.

Checking locally, this is sufficient to unblock the Dask builds, but I notice that there are still some issues around the dask pinning in dask-sql's builds:

https://gpuci.gpuopenanalytics.com/job/dask/job/dask-build-environment/job/branch/job/dask-build-env-main/BUILD_NAME=dask_sql,CUDA_VER=11.8.0,LINUX_VER=ubuntu20.04,PYTHON_VER=3.10,RAPIDS_VER=24.02/1058/console

With rapids-dask-dependency and dask-sql both pinned to 2024.1.1 for now, we shouldn't need to toy around with dask's pinning at all, so can make that change in this PR so we can wrap up all the build issues at once

@charlesbluca charlesbluca changed the title Unpin numpy in Dask CI environment Unpin numpy in Dask environment, leave dask pinned in Dask-SQL environment Mar 14, 2024
@jakirkham
Copy link
Member

Thanks Charles! 🙏

Makes sense

Do we want a NumPy 2 upper bound here?

@charlesbluca
Copy link
Member Author

Do we want a NumPy 2 upper bound here?

I don't think we need it to unblock, as right now just loosening the explicit pinning means we should fall back on whatever upper bound gets set by RAPIDS, though it might be worth adding pinnings to the environments here on some core packages like numpy, pandas, etc. that we can align with RAPIDS pinnings as they change to make it immediately obvious when things have stopped working (versus right now when we're only alerted when conda is unable to solve some environment).

Expect to have more time to think about this post-GTC, but can file an issue here around this work for now to follow up on in coming weeks

@jakirkham
Copy link
Member

Ok sure that makes. Happy to go with what we have here

Let's raise an issue to follow up on RAPIDS alignment pins for numpy, pandas, etc.

@charlesbluca
Copy link
Member Author

Cool, did a quick write up of my initial thoughts here #89

@jakirkham
Copy link
Member

Looks good. Thanks Charles! 🙏

Ready to merge?

@charlesbluca
Copy link
Member Author

Yup! Can follow up here once the builds are rerun and succeed, thanks for the help @jakirkham!

@charlesbluca charlesbluca merged commit 0ae3504 into rapidsai:main Mar 14, 2024
@jakirkham
Copy link
Member

Thanks Charles! 🙏

@charlesbluca
Copy link
Member Author

Looks like that unblocked builds - thanks! 🙏🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants