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

Jump to bottom

build wheels without build isolation #1473

Open
wants to merge 28 commits into
base: branch-24.12
Choose a base branch
from
Open

build wheels without build isolation #1473

Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
b983143
test wheel-build sccache fixes
jameslamb Oct 11, 2024
9e2ceef
fiddle with options
jameslamb Oct 11, 2024
8c747f5
run fewer build jobs
jameslamb Oct 11, 2024
7cf37ee
build with --no-build-isolation
jameslamb Oct 11, 2024
4477b70
paths
jameslamb Oct 11, 2024
d6f05c6
more paths
jameslamb Oct 11, 2024
d18c3f7
Merge branch 'wheel-build-sccache' of github.com:jameslamb/cuspatial …
jameslamb Oct 11, 2024
6c64e73
restore all CI
jameslamb Oct 11, 2024
cbfaf85
only pre-install build backend
jameslamb Oct 14, 2024
9744bcf
Merge branch 'wheel-build-sccache' of github.com:jameslamb/cuspatial …
jameslamb Oct 14, 2024
8256e35
only run wheel builds
jameslamb Oct 14, 2024
28e34eb
GHA fix
jameslamb Oct 14, 2024
8251aaa
go back to installing everything
jameslamb Oct 14, 2024
130e761
re-enable all CI
jameslamb Oct 14, 2024
29b005e
move PIP_CONSTRAINT stuff back, remove 'declare'
jameslamb Oct 14, 2024
fc8d575
Merge branch 'wheel-build-sccache' of github.com:jameslamb/cuspatial …
jameslamb Oct 14, 2024
ec06a4a
Merge branch 'branch-24.12' into wheel-build-sccache
bdice Oct 16, 2024
cb6b07d
only use an un-isolated build for libcuspatial
jameslamb Oct 16, 2024
75eaf12
Merge branch 'wheel-build-sccache' of github.com:jameslamb/cuspatial …
jameslamb Oct 16, 2024
7b87cd6
Merge branch 'branch-24.12' of github.com:rapidsai/cuspatial into whe…
jameslamb Oct 17, 2024
f59a067
test with new rapids-dependency-file-generator
jameslamb Oct 17, 2024
1199e31
enable all CI
jameslamb Oct 17, 2024
5e54225
shell variable
jameslamb Oct 17, 2024
9f979b5
actually turn off build isolation
jameslamb Oct 17, 2024
aa62971
more fiddling with PIP_NO_BUILD_ISOLATION
jameslamb Oct 17, 2024
b4cd177
ugh ok yeah it really is PIP_NO_BUILD_ISOLATION=0
jameslamb Oct 17, 2024
69fa9e8
try more libcuspatial builds
jameslamb Oct 17, 2024
8b841c2
restore all CI
jameslamb Oct 17, 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
10 changes: 9 additions & 1 deletion ci/build_wheel.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,15 @@ RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen ${RAPIDS_CUDA_VERSION})"

cd "${package_dir}"

python -m pip wheel . -w dist -vvv --no-deps --disable-pip-version-check
rapids-logger "Building '${package_name}' wheel"
python -m pip wheel \
-w dist \
-v \
--no-deps \
--disable-pip-version-check \
.

sccache --show-adv-stats
Copy link
Member Author

Choose a reason for hiding this comment

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

Anticipating a review comment like "is this left over from debugging?"... I've intentionally left this in. I think it's generally useful to print the sccache stats in these CI logs, as they could provide a hint when you experience issues like CI timeouts or out-of-memory.

This only adds 27 lines to the logs, like this:

Compile requests                     28
Compile requests executed            28
Cache hits                           28
Cache hits (c [gcc])                  2
Cache hits (c++ [gcc])                2
Cache hits (cuda [nvcc])             24
Cache misses                          0
Cache timeouts                        0
Cache read errors                     0
Forced recaches                       0
Cache write errors                    0
Compilation failures                  0
Cache errors                          0
Non-cacheable compilations            0
Non-cacheable calls                   0
Non-compilation calls                 0
Unsupported compiler calls            0
Average cache write               0.000 s
Average compiler                  0.000 s
Average cache read hit            0.077 s
Failed distributed compilations       0
Cache location                  s3, name: rapids-sccache-east, prefix: /
Version (client)                0.7.7

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this is good IMO. I'm also going to add this kind of thing into the telemetry.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, I want us to start printing sccache in more jobs so that we can proactively identify these kinds of issues.


mkdir -p final_dist
python -m auditwheel repair \
Expand Down
32 changes: 31 additions & 1 deletion ci/build_wheel_libcuspatial.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,34 @@

set -euo pipefail

ci/build_wheel.sh libcuspatial python/libcuspatial cpp
package_name="libcuspatial"

git clone \
--branch multiple-file-keys \
https://github.com/jameslamb/dependency-file-generator.git \
/tmp/rapids-dependency-file-generator

pip uninstall --yes rapids-dependency-file-generator
pip install /tmp/rapids-dependency-file-generator

Comment on lines +8 to +15
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
git clone \
--branch multiple-file-keys \
https://github.com/jameslamb/dependency-file-generator.git \
/tmp/rapids-dependency-file-generator
pip uninstall --yes rapids-dependency-file-generator
pip install /tmp/rapids-dependency-file-generator

TODO: remove this before merging

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
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 I think this is working.

On the latest build on branch-24.12, you can see log lines about installing build dependencies and creating a temporary build directory.

Created temporary directory: /tmp/pip-wheel-ndbc3klu
Created temporary directory: /tmp/pip-ephem-wheel-cache-blfbvohw
Looking in indexes: https://pypi.org/simple, https://pypi.anaconda.org/rapidsai-wheels-nightly/simple, https://pypi.nvidia.com/
Processing /__w/cuspatial/cuspatial/python/libcuspatial
  Added file:///__w/cuspatial/cuspatial/python/libcuspatial to build tracker '/tmp/pip-build-tracker-vn6jabrn'
  Created temporary directory: /tmp/pip-build-env-64dc7d88
  Running command pip subprocess to install build dependencies
  Installing build dependencies: started

(build link)

On this PR, none of that.

Looking in indexes: https://pypi.org/simple, https://pypi.anaconda.org/rapidsai-wheels-nightly/simple, https://pypi.nvidia.com/
Processing /__w/cuspatial/cuspatial/python/libcuspatial
  Running command Preparing metadata (pyproject.toml)
  Preparing metadata (pyproject.toml): started

(build link)

Copy link
Member Author

Choose a reason for hiding this comment

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

I mention that because it looks to me like we're not getting sccache hits.

Successfully built libcuspatial-cu11
Compile requests                      28
Compile requests executed             28
Cache hits                             4
Cache hits (c [gcc])                   2

There hasn't been a new rmm commit merged since yesterday, and the last new cudf commit merged was an hour ago.

Gonna try another build to double-check this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah cool ok, I just did another build and saw all cache hits. Must have just gotten unlucky before with new cudf changes being committed and new nightlies appearing.

Compile requests                     28
Compile requests executed            28
Cache hits                           28
Cache hits (c [gcc])                  2
Cache hits (c++ [gcc])                2
Cache hits (cuda [nvcc])             24

(build link)


ci/build_wheel.sh "${package_name}" python/libcuspatial cpp
Loading