-
Notifications
You must be signed in to change notification settings - Fork 198
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
introduce libraft wheels #2531
introduce libraft wheels #2531
Changes from 34 commits
24a4db8
c0f059c
d116417
eb17735
d317235
679d289
51e7041
6d38c19
717df6f
d815242
11168a1
46bfcfa
97f5d31
f492d59
ad13d08
6bf5eba
9783244
d0b6385
ee99702
0ac23f4
48c98b8
2070b11
345f0e5
4c657b2
dbbfbb5
6deeb51
c6579e1
1fd7603
a1a905d
c224e27
76d788b
d275c99
b2a5107
4b793be
002e824
020ce6a
1d9e1e9
a800691
6635330
0d6597b
93f39fb
38aede9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -66,7 +66,30 @@ jobs: | |
node_type: "gpu-v100-latest-1" | ||
run_script: "ci/build_docs.sh" | ||
sha: ${{ inputs.sha }} | ||
wheel-build-libraft: | ||
secrets: inherit | ||
uses: rapidsai/shared-workflows/.github/workflows/[email protected] | ||
with: | ||
build_type: ${{ inputs.build_type || 'branch' }} | ||
branch: ${{ inputs.branch }} | ||
sha: ${{ inputs.sha }} | ||
date: ${{ inputs.date }} | ||
script: ci/build_wheel_libraft.sh | ||
# build for every combination of arch and CUDA version, but only for the latest Python | ||
matrix_filter: group_by([.ARCH, (.CUDA_VER|split(".")|map(tonumber)|.[0])]) | map(max_by(.PY_VER|split(".")|map(tonumber))) | ||
wheel-publish-libraft: | ||
needs: wheel-build-libraft | ||
secrets: inherit | ||
uses: rapidsai/shared-workflows/.github/workflows/[email protected] | ||
with: | ||
build_type: ${{ inputs.build_type || 'branch' }} | ||
branch: ${{ inputs.branch }} | ||
sha: ${{ inputs.sha }} | ||
date: ${{ inputs.date }} | ||
package-name: libraft | ||
package-type: cpp | ||
wheel-build-pylibraft: | ||
needs: wheel-publish-libraft | ||
secrets: inherit | ||
uses: rapidsai/shared-workflows/.github/workflows/[email protected] | ||
with: | ||
|
@@ -85,6 +108,7 @@ jobs: | |
sha: ${{ inputs.sha }} | ||
date: ${{ inputs.date }} | ||
package-name: pylibraft | ||
package-type: python | ||
wheel-build-raft-dask: | ||
secrets: inherit | ||
uses: rapidsai/shared-workflows/.github/workflows/[email protected] | ||
|
@@ -104,3 +128,4 @@ jobs: | |
sha: ${{ inputs.sha }} | ||
date: ${{ inputs.date }} | ||
package-name: raft_dask | ||
package-type: python |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,11 +21,12 @@ jobs: | |
- conda-python-build | ||
- conda-python-tests | ||
- docs-build | ||
- wheel-build-libraft | ||
- wheel-build-pylibraft | ||
- wheel-tests-pylibraft | ||
- wheel-build-raft-dask | ||
- wheel-tests-raft-dask | ||
- devcontainer | ||
# - devcontainer | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TODO: Put this back before merging. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just pushed 93f39fb restoring the devcontainer builds. They'll still fail because the manifest in devcontainers needs to be updated. Summarizing plan we agreed to offline:
|
||
secrets: inherit | ||
uses: rapidsai/shared-workflows/.github/workflows/[email protected] | ||
if: always() | ||
|
@@ -116,10 +117,22 @@ jobs: | |
arch: "amd64" | ||
container_image: "rapidsai/ci-conda:latest" | ||
run_script: "ci/build_docs.sh" | ||
wheel-build-pylibraft: | ||
wheel-build-libraft: | ||
needs: checks | ||
secrets: inherit | ||
uses: rapidsai/shared-workflows/.github/workflows/[email protected] | ||
with: | ||
build_type: pull-request | ||
branch: ${{ inputs.branch }} | ||
sha: ${{ inputs.sha }} | ||
date: ${{ inputs.date }} | ||
script: ci/build_wheel_libraft.sh | ||
# build for every combination of arch and CUDA version, but only for the latest Python | ||
matrix_filter: group_by([.ARCH, (.CUDA_VER|split(".")|map(tonumber)|.[0])]) | map(max_by(.PY_VER|split(".")|map(tonumber))) | ||
wheel-build-pylibraft: | ||
needs: [checks, wheel-build-libraft] | ||
secrets: inherit | ||
uses: rapidsai/shared-workflows/.github/workflows/[email protected] | ||
with: | ||
build_type: pull-request | ||
script: ci/build_wheel_pylibraft.sh | ||
|
@@ -132,7 +145,7 @@ jobs: | |
build_type: pull-request | ||
script: ci/test_wheel_pylibraft.sh | ||
wheel-build-raft-dask: | ||
needs: wheel-tests-pylibraft | ||
needs: [checks, wheel-build-libraft] | ||
vyasr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
secrets: inherit | ||
uses: rapidsai/shared-workflows/.github/workflows/[email protected] | ||
with: | ||
|
@@ -146,13 +159,13 @@ jobs: | |
with: | ||
build_type: pull-request | ||
script: ci/test_wheel_raft_dask.sh | ||
devcontainer: | ||
secrets: inherit | ||
uses: rapidsai/shared-workflows/.github/workflows/[email protected] | ||
with: | ||
arch: '["amd64"]' | ||
cuda: '["12.5"]' | ||
build_command: | | ||
sccache -z; | ||
build-all -DBUILD_PRIMS_BENCH=ON --verbose; | ||
sccache -s; | ||
# devcontainer: | ||
# secrets: inherit | ||
# uses: rapidsai/shared-workflows/.github/workflows/[email protected] | ||
# with: | ||
# arch: '["amd64"]' | ||
# cuda: '["12.5"]' | ||
# build_command: | | ||
# sccache -z; | ||
# build-all -DBUILD_PRIMS_BENCH=ON --verbose; | ||
# sccache -s; |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,43 @@ | ||||||
#!/bin/bash | ||||||
# Copyright (c) 2024, NVIDIA CORPORATION. | ||||||
|
||||||
set -euo pipefail | ||||||
|
||||||
package_name="libraft" | ||||||
package_dir="python/libraft" | ||||||
|
||||||
rapids-logger "Generating build requirements" | ||||||
matrix_selectors="cuda=${RAPIDS_CUDA_VERSION%.*};arch=$(arch);py=${RAPIDS_PY_VERSION};cuda_suffixed=true" | ||||||
|
||||||
rapids-dependency-file-generator \ | ||||||
--output requirements \ | ||||||
--file-key "py_build_${package_name}" \ | ||||||
--file-key "py_rapids_build_${package_name}" \ | ||||||
--matrix "${matrix_selectors}" \ | ||||||
| tee /tmp/requirements-build.txt | ||||||
|
||||||
rapids-logger "Installing build requirements" | ||||||
python -m pip install \ | ||||||
-v \ | ||||||
--prefer-binary \ | ||||||
-r /tmp/requirements-build.txt | ||||||
|
||||||
# build with '--no-build-isolation', for better sccache hit rate | ||||||
# 0 really means "add --no-build-isolation" (ref: https://github.com/pypa/pip/issues/5735) | ||||||
export PIP_NO_BUILD_ISOLATION=0 | ||||||
|
||||||
RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen ${RAPIDS_CUDA_VERSION})" | ||||||
|
||||||
case "${RAPIDS_CUDA_VERSION}" in | ||||||
12.*) | ||||||
EXTRA_CMAKE_ARGS=";-DUSE_CUDA_MATH_WHEELS=ON" | ||||||
;; | ||||||
11.*) | ||||||
EXTRA_CMAKE_ARGS=";-DUSE_CUDA_MATH_WHEELS=OFF" | ||||||
;; | ||||||
esac | ||||||
|
||||||
export SKBUILD_CMAKE_ARGS="-DDETECT_CONDA_ENV=OFF${EXTRA_CMAKE_ARGS}" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Honestly I don't think we need to be setting this explicitly almost ever. All our wheel builds occur in environments where we don't have a conda environment. I'd prefer to remove this altogether. If we did keep it though, minor nit: I would prefer to have an explicit semicolon here rather than embedding it in each incantation of
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok that's fine, I can try removing it. Both of those patterns (setting There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removing it worked for me locally. Did that in 1d9e1e9#diff-bf2a37f5233778a838d488772273c05401ae172253b19533bc07e7ca958ba271. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool thanks. I'll defer to you on when and how you want to propagate these changes through the rest of RAPIDS. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Put up this tracking issue: rapidsai/build-planning#136 I'm going to try to do that in the next day or 2, to try to get it into 25.02. |
||||||
|
||||||
ci/build_wheel.sh libraft ${package_dir} cpp | ||||||
ci/validate_wheel.sh ${package_dir} final_dist libraft |
vyasr marked this conversation as resolved.
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The job downloads from s3, so really this can happen after the libraft build is complete and doesn't have to wait for publish, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, good point!!!
I'll change that here, and put up PRs across RAPIDS with changes like that.
Across RAPIDS, we have
wheel-build-*
depending onwheel-publish-*
of its dependencies.https://github.com/rapidsai/cudf/blob/e272f1e60978801809945c59240dfb02f0aa3b07/.github/workflows/build.yaml#L92-L93
https://github.com/rapidsai/cugraph/blob/6b5db94d9b6ebee41577222d230a36675d3e3ff4/.github/workflows/build.yaml#L90-L91
That extra parallelism should reduce the end-to-end time for publishing nightlies 😁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice thanks! I'll let you handle doing this elsewhere as time permits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No prob, no prob. I'm gonna wait until we've finished rapidsai/cugraph#4804, in case that uncovers other this-should-change-across-all-of-RAPIDS things, then do a sweep of 1 PR per repo with these changes where necessary.
I'm keeping a list, so far I have:
*-build
not*-publish
*-build
not*-tests
-DDETECT_CONDA_ENV=OFF
from wheel-building scriptsrapids-cmake
's preferred CMake formatting (introduce libraft wheels #2531 (comment))