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

add support for python 3.11 #176

Merged
merged 10 commits into from
Feb 26, 2024
Merged

add support for python 3.11 #176

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ac20007
add support for python 3.11
jameslamb Feb 20, 2024
dbfc993
switch order
jameslamb Feb 20, 2024
c32c724
add TODO comment
jameslamb Feb 20, 2024
ed8590a
more TODO comments
jameslamb Feb 20, 2024
ea9c6cd
re-organizing
jameslamb Feb 21, 2024
e678683
Merge branch 'branch-24.04' into python-3.11
jameslamb Feb 22, 2024
baf15bd
re-shuffle matrices
jameslamb Feb 22, 2024
d227937
Merge branch 'branch-24.04' into python-3.11
jameslamb Feb 22, 2024
b15f36c
Apply suggestions from code review
jameslamb Feb 23, 2024
115ad5a
make nightly a superset of PRs
jameslamb Feb 23, 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
6 changes: 6 additions & 0 deletions .github/workflows/conda-python-build.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,15 @@ jobs:
- { ARCH: 'amd64', PY_VER: '3.9', CUDA_VER: '12.2.2', LINUX_VER: 'ubuntu22.04' }
- { ARCH: 'amd64', PY_VER: '3.10', CUDA_VER: '11.8.0', LINUX_VER: 'ubuntu22.04' }
- { ARCH: 'amd64', PY_VER: '3.10', CUDA_VER: '12.2.2', LINUX_VER: 'ubuntu22.04' }
- { ARCH: 'amd64', PY_VER: '3.11', CUDA_VER: '11.8.0', LINUX_VER: 'ubuntu22.04' }
- { ARCH: 'amd64', PY_VER: '3.11', CUDA_VER: '12.2.2', LINUX_VER: 'ubuntu22.04' }
# arm64
- { ARCH: 'arm64', PY_VER: '3.9', CUDA_VER: '11.8.0', LINUX_VER: 'ubuntu22.04' }
- { ARCH: 'arm64', PY_VER: '3.9', CUDA_VER: '12.2.2', LINUX_VER: 'ubuntu22.04' }
- { ARCH: 'arm64', PY_VER: '3.10', CUDA_VER: '11.8.0', LINUX_VER: 'ubuntu22.04' }
- { ARCH: 'arm64', PY_VER: '3.10', CUDA_VER: '12.2.2', LINUX_VER: 'ubuntu22.04' }
- { ARCH: 'arm64', PY_VER: '3.11', CUDA_VER: '11.8.0', LINUX_VER: 'ubuntu22.04' }
- { ARCH: 'arm64', PY_VER: '3.11', CUDA_VER: '12.2.2', LINUX_VER: 'ubuntu22.04' }
"

echo "MATRIX=$(
Expand All @@ -79,6 +83,8 @@ jobs:
fail-fast: false
matrix: ${{ fromJSON(needs.compute-matrix.outputs.MATRIX) }}
runs-on: "linux-${{ matrix.ARCH }}-${{ inputs.node_type }}"
# TODO: remove 'continue-on-error' before closing https://github.com/rapidsai/build-planning/issues/3
continue-on-error: ${{ matrix.PY_VER == '3.11' }}
jameslamb marked this conversation as resolved.
Show resolved Hide resolved
env:
RAPIDS_ARTIFACTS_DIR: ${{ github.workspace }}/artifacts
container:
Expand Down
10 changes: 6 additions & 4 deletions .github/workflows/conda-python-tests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,21 +73,21 @@ jobs:
# amd64
- { ARCH: 'amd64', PY_VER: '3.9', CUDA_VER: '11.4.3', LINUX_VER: 'centos7', GPU: 'v100', DRIVER: 'earliest' }
- { ARCH: 'amd64', PY_VER: '3.10', CUDA_VER: '11.8.0', LINUX_VER: 'ubuntu22.04', GPU: 'v100', DRIVER: 'latest' }
- { ARCH: 'amd64', PY_VER: '3.10', CUDA_VER: '12.2.2', LINUX_VER: 'ubuntu22.04', GPU: 'v100', DRIVER: 'latest' }
- { ARCH: 'amd64', PY_VER: '3.11', CUDA_VER: '12.2.2', LINUX_VER: 'ubuntu22.04', GPU: 'v100', DRIVER: 'latest' }
# arm64
- { ARCH: 'arm64', PY_VER: '3.10', CUDA_VER: '12.0.1', LINUX_VER: 'ubuntu22.04', GPU: 'a100', DRIVER: 'latest' }
nightly:
# amd64
- { ARCH: 'amd64', PY_VER: '3.9', CUDA_VER: '11.4.3', LINUX_VER: 'centos7', GPU: 'v100', DRIVER: 'earliest' }
- { ARCH: 'amd64', PY_VER: '3.9', CUDA_VER: '11.4.3', LINUX_VER: 'ubuntu20.04', GPU: 'v100', DRIVER: 'latest' }
- { ARCH: 'amd64', PY_VER: '3.10', CUDA_VER: '11.8.0', LINUX_VER: 'rockylinux8', GPU: 'v100', DRIVER: 'latest' }
- { ARCH: 'amd64', PY_VER: '3.10', CUDA_VER: '12.0.1', LINUX_VER: 'ubuntu22.04', GPU: 'v100', DRIVER: 'latest' }
- { ARCH: 'amd64', PY_VER: '3.10', CUDA_VER: '12.2.2', LINUX_VER: 'ubuntu22.04', GPU: 'v100', DRIVER: 'latest' }
- { ARCH: 'amd64', PY_VER: '3.11', CUDA_VER: '12.0.1', LINUX_VER: 'ubuntu22.04', GPU: 'v100', DRIVER: 'latest' }
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like we're not testing amd64 + latest CUDA + latest Python. I want that specific configuration to be in the matrix since it is a helpful edge case. This is what I would have expected (swap CUDA versions):

Suggested change
- { ARCH: 'amd64', PY_VER: '3.10', CUDA_VER: '12.2.2', LINUX_VER: 'ubuntu22.04', GPU: 'v100', DRIVER: 'latest' }
- { ARCH: 'amd64', PY_VER: '3.11', CUDA_VER: '12.0.1', LINUX_VER: 'ubuntu22.04', GPU: 'v100', DRIVER: 'latest' }
- { ARCH: 'amd64', PY_VER: '3.10', CUDA_VER: '12.0.1', LINUX_VER: 'ubuntu22.04', GPU: 'v100', DRIVER: 'latest' }
- { ARCH: 'amd64', PY_VER: '3.11', CUDA_VER: '12.2.2', LINUX_VER: 'ubuntu22.04', GPU: 'v100', DRIVER: 'latest' }

Copy link
Member Author

Choose a reason for hiding this comment

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

Promise I didn't ignore your previous comment asking for that combination.

On the current state of this branch, that exact combination is being tested on PRs for conda-python-tests workflow, which is why I didn't also add it to the nightly matrix.

Does that change you opinion on this suggestion?

Or do you think it's desirable to have that combination in both places because we don't enforce that PRs have to be up to date with the target branch before merging?

Copy link
Contributor

@bdice bdice Feb 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Or do you think it's desirable to have that combination in both places because we don't enforce that PRs have to be up to date with the target branch before merging?

(Yes, and...) it's very helpful for the nightly matrix to be a strict superset of the PR matrix. It's hard to test things that fail in nightlies unless we have a corresponding entry in the PR matrix (otherwise we're stuck merging and waiting for another nightly). This comes at a slight cost to overall matrix-coverage-over-time but it really helps the sanity of developers. 😅

And even if the PR matrix is not a strict subset of the nightly matrix -- it helps for it to be pretty close. I definitely want the "endpoints" like latest-everything to be in both matrices.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok got it! That was not a constraint I was considering when I came up with the proposal here. I was leaning toward denser coverage of the matrix over time in exchange for a higher risk of "this PR passed but nightly builds failed", since I saw that the nightly matrix was already so much bigger than the PR one.

I've added a comment about that at rapidsai/build-planning#5 (comment) ... think it's another example of the type of constraint we should be able to enforce with automation here, to reduce the effort it requires for you and others to review PRs like this one.

I'll go over all the matrices touched in this PR and make sure this constraint's being met, will request another review when that's done.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just pushed 115ad5a. As of that commit, I believe this branch is meeting these constraints for test job matrices:

  • at least 1 PR job for every Python version in (3.9, 3.10, 3.11)
  • at least 1 PR job for every CUDA version in (11.8.0, 12.0.1, 12.2.2)
  • at least 1 PR job for every architecture in (amd64, arm64)
  • am64 + latest CUDA + latest Python + latest Ubuntu present in both PR and nightly
  • nightly is a strict superset of what's tested on PRs
  • no net change to the number of test jobs on PRs or nightly, relative to the current state of branch-24.04
  • as-close-as-possible-to-equal spread of test jobs across Python versions

Since that commit only changed CUDA_VER and LINUX_VER, the table from #176 (comment) is unchanged.

# arm64
- { ARCH: 'arm64', PY_VER: '3.9', CUDA_VER: '11.4.3', LINUX_VER: 'ubuntu20.04', GPU: 'a100', DRIVER: 'latest' }
- { ARCH: 'arm64', PY_VER: '3.10', CUDA_VER: '11.8.0', LINUX_VER: 'ubuntu22.04', GPU: 'a100', DRIVER: 'latest' }
- { ARCH: 'arm64', PY_VER: '3.10', CUDA_VER: '12.0.1', LINUX_VER: 'ubuntu22.04', GPU: 'a100', DRIVER: 'latest' }
- { ARCH: 'arm64', PY_VER: '3.10', CUDA_VER: '12.2.2', LINUX_VER: 'rockylinux8', GPU: 'a100', DRIVER: 'latest' }
- { ARCH: 'arm64', PY_VER: '3.11', CUDA_VER: '11.8.0', LINUX_VER: 'ubuntu22.04', GPU: 'a100', DRIVER: 'latest' }
- { ARCH: 'arm64', PY_VER: '3.11', CUDA_VER: '12.2.2', LINUX_VER: 'rockylinux8', GPU: 'a100', DRIVER: 'latest' }
"

TEST_MATRIX=$(yq -n 'env(MATRICES) | .[strenv(BUILD_TYPE)]')
Expand All @@ -103,6 +103,8 @@ jobs:
fail-fast: false
matrix: ${{ fromJSON(needs.compute-matrix.outputs.MATRIX) }}
runs-on: "linux-${{ matrix.ARCH }}-gpu-${{ matrix.GPU }}-${{ matrix.DRIVER }}-1"
# TODO: remove 'continue-on-error' before closing https://github.com/rapidsai/build-planning/issues/3
continue-on-error: ${{ matrix.PY_VER == '3.11' }}
env:
RAPIDS_COVERAGE_DIR: ${{ github.workspace }}/coverage-results
RAPIDS_TESTS_DIR: ${{ github.workspace }}/test-results
Expand Down
12 changes: 10 additions & 2 deletions .github/workflows/wheels-build.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,19 +92,25 @@ jobs:
- { ARCH: 'amd64', PY_VER: '3.9', CUDA_VER: '12.2.2', LINUX_VER: 'centos7' }
- { ARCH: 'amd64', PY_VER: '3.10', CUDA_VER: '11.8.0', LINUX_VER: 'centos7' }
- { ARCH: 'amd64', PY_VER: '3.10', CUDA_VER: '12.2.2', LINUX_VER: 'centos7' }
- { ARCH: 'amd64', PY_VER: '3.11', CUDA_VER: '11.8.0', LINUX_VER: 'centos7' }
- { ARCH: 'amd64', PY_VER: '3.11', CUDA_VER: '12.2.2', LINUX_VER: 'centos7' }
# arm64
- { ARCH: 'arm64', PY_VER: '3.9', CUDA_VER: '11.8.0', LINUX_VER: 'rockylinux8' }
- { ARCH: 'arm64', PY_VER: '3.9', CUDA_VER: '12.2.2', LINUX_VER: 'rockylinux8' }
- { ARCH: 'arm64', PY_VER: '3.10', CUDA_VER: '11.8.0', LINUX_VER: 'rockylinux8' }
- { ARCH: 'arm64', PY_VER: '3.10', CUDA_VER: '12.2.2', LINUX_VER: 'rockylinux8' }
- { ARCH: 'arm64', PY_VER: '3.11', CUDA_VER: '11.8.0', LINUX_VER: 'rockylinux8' }
- { ARCH: 'arm64', PY_VER: '3.11', CUDA_VER: '12.2.2', LINUX_VER: 'rockylinux8' }
"

export NEW_MANYLINUX_MATRIX="
# amd64
- { ARCH: 'amd64', PY_VER: '3.9', CUDA_VER: '11.8.0', LINUX_VER: 'rockylinux8' }
- { ARCH: 'amd64', PY_VER: '3.9', CUDA_VER: '12.2.2', LINUX_VER: 'rockylinux8' }
- { ARCH: 'amd64', PY_VER: '3.10', CUDA_VER: '11.8.0', LINUX_VER: 'rockylinux8' }
- { ARCH: 'amd64', PY_VER: '3.9', CUDA_VER: '12.2.2', LINUX_VER: 'rockylinux8' }
- { ARCH: 'amd64', PY_VER: '3.10', CUDA_VER: '12.2.2', LINUX_VER: 'rockylinux8' }
- { ARCH: 'amd64', PY_VER: '3.10', CUDA_VER: '12.2.2', LINUX_VER: 'rockylinux8' }
- { ARCH: 'amd64', PY_VER: '3.11', CUDA_VER: '11.8.0', LINUX_VER: 'rockylinux8' }
- { ARCH: 'amd64', PY_VER: '3.11', CUDA_VER: '12.2.2', LINUX_VER: 'rockylinux8' }
"

if ${{ inputs.build-2_28-wheels == 'true' }}; then
Expand All @@ -122,6 +128,8 @@ jobs:
strategy:
matrix: ${{ fromJSON(needs.compute-matrix.outputs.MATRIX) }}
runs-on: "linux-${{ matrix.ARCH }}-${{ inputs.node_type }}"
# TODO: remove 'continue-on-error' before closing https://github.com/rapidsai/build-planning/issues/3
continue-on-error: ${{ matrix.PY_VER == '3.11' }}
env:
RAPIDS_ARTIFACTS_DIR: ${{ github.workspace }}/artifacts
container:
Expand Down
12 changes: 7 additions & 5 deletions .github/workflows/wheels-test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,22 +76,22 @@ jobs:
pull-request:
# amd64
- { ARCH: 'amd64', PY_VER: '3.9', CUDA_VER: '11.8.0', LINUX_VER: 'ubuntu18.04', gpu: 'v100', driver: 'latest' }
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that when we drop CentOS 7 (glibc 2.17) in RAPIDS 24.06, we will also have to drop all our test configurations with Ubuntu 18.04 (glibc 2.27). Our new minimum glibc version will be 2.28.

We have already dropped Ubuntu 18.04 support but we will need to follow-up and remove it from our CI matrices once our minimum glibc is bumped.

All this is now documented here: rapidsai/build-planning#23

- { ARCH: 'amd64', PY_VER: '3.10', CUDA_VER: '12.2.2', LINUX_VER: 'ubuntu20.04', gpu: 'v100', driver: 'latest' }
- { ARCH: 'amd64', PY_VER: '3.11', CUDA_VER: '12.2.2', LINUX_VER: 'ubuntu22.04', gpu: 'v100', driver: 'latest' }
# arm64
- { ARCH: 'arm64', PY_VER: '3.10', CUDA_VER: '12.0.1', LINUX_VER: 'ubuntu20.04', gpu: 'a100', driver: 'latest' }
nightly:
# amd64
- { ARCH: 'amd64', PY_VER: '3.9', CUDA_VER: '11.8.0', LINUX_VER: 'ubuntu18.04', gpu: 'v100', driver: 'latest' }
- { ARCH: 'amd64', PY_VER: '3.9', CUDA_VER: '12.0.1', LINUX_VER: 'ubuntu18.04', gpu: 'v100', driver: 'latest' }
- { ARCH: 'amd64', PY_VER: '3.9', CUDA_VER: '12.2.2', LINUX_VER: 'ubuntu22.04', gpu: 'v100', driver: 'latest' }
- { ARCH: 'amd64', PY_VER: '3.10', CUDA_VER: '11.8.0', LINUX_VER: 'ubuntu18.04', gpu: 'v100', driver: 'latest' }
- { ARCH: 'amd64', PY_VER: '3.10', CUDA_VER: '12.0.1', LINUX_VER: 'ubuntu20.04', gpu: 'v100', driver: 'latest' }
- { ARCH: 'amd64', PY_VER: '3.10', CUDA_VER: '12.2.2', LINUX_VER: 'ubuntu20.04', gpu: 'v100', driver: 'latest' }
- { ARCH: 'amd64', PY_VER: '3.11', CUDA_VER: '12.0.1', LINUX_VER: 'ubuntu18.04', gpu: 'v100', driver: 'latest' }
- { ARCH: 'amd64', PY_VER: '3.11', CUDA_VER: '12.2.2', LINUX_VER: 'ubuntu22.04', gpu: 'v100', driver: 'latest' }
# arm64
- { ARCH: 'arm64', PY_VER: '3.9', CUDA_VER: '11.8.0', LINUX_VER: 'ubuntu20.04', gpu: 'a100', driver: 'latest' }
- { ARCH: 'arm64', PY_VER: '3.9', CUDA_VER: '12.0.1', LINUX_VER: 'ubuntu20.04', gpu: 'a100', driver: 'latest' }
- { ARCH: 'arm64', PY_VER: '3.10', CUDA_VER: '11.8.0', LINUX_VER: 'ubuntu20.04', gpu: 'a100', driver: 'latest' }
- { ARCH: 'arm64', PY_VER: '3.10', CUDA_VER: '12.0.1', LINUX_VER: 'ubuntu20.04', gpu: 'a100', driver: 'latest' }
- { ARCH: 'arm64', PY_VER: '3.10', CUDA_VER: '12.2.2', LINUX_VER: 'ubuntu22.04', gpu: 'a100', driver: 'latest' }
- { ARCH: 'arm64', PY_VER: '3.11', CUDA_VER: '11.8.0', LINUX_VER: 'ubuntu20.04', gpu: 'a100', driver: 'latest' }
"

TEST_MATRIX=$(yq -n 'env(MATRICES) | .[strenv(BUILD_TYPE)]')
Expand All @@ -111,6 +111,8 @@ jobs:
strategy:
matrix: ${{ fromJSON(needs.compute-matrix.outputs.MATRIX) }}
runs-on: "linux-${{ matrix.ARCH }}-gpu-${{ matrix.gpu }}-${{ matrix.driver }}-1"
# TODO: remove 'continue-on-error' before closing https://github.com/rapidsai/build-planning/issues/3
continue-on-error: ${{ matrix.PY_VER == '3.11' }}
container:
image: "rapidsai/citestwheel:cuda${{ matrix.CUDA_VER }}-${{ matrix.LINUX_VER }}-py${{ matrix.PY_VER }}"
options: ${{ inputs.container-options }}
Expand Down