build wheels on CI #619
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| #!/bin/bash | ||
| # Copyright (c) 2023, NVIDIA CORPORATION. | ||
|
|
||
| set -euo pipefail | ||
|
|
||
| package_name="cucim" | ||
| package_dir="python/cucim" | ||
| package_src_dir="${package_dir}/src/${package_name}" | ||
|
|
||
| CMAKE_BUILD_TYPE="release" | ||
|
There was a problem hiding this comment. thought: I was going to ask if this was really necessary since it's CMake's default, but now I see that this is for the There was a problem hiding this comment. There is a default, but it is "debug" rather than release! Could potentially change it if there is no objection from @gigony local subcommand="${1:-all}"
local build_type="${2:-debug}"There was a problem hiding this comment. I'll leave that up to you two decide. It does seem odd to me that the default would be a debug build though. |
||
|
|
||
| source rapids-configure-sccache | ||
| source rapids-date-string | ||
|
|
||
| version=$(rapids-generate-version) | ||
| commit=$(git rev-parse HEAD) | ||
|
|
||
| # for CMake VERSION_CPP need to truncate any trailing 'a' | ||
| version_cpp=${version%a*} | ||
|
|
||
| RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen ${RAPIDS_CUDA_VERSION})" | ||
|
|
||
| # Patch project metadata files to include the CUDA version suffix and version override. | ||
| pyproject_file="${package_dir}/pyproject.toml" | ||
|
|
||
| PACKAGE_CUDA_SUFFIX="-${RAPIDS_PY_CUDA_SUFFIX}" | ||
| # update package name to have the cuda suffix | ||
| sed -i "s/name = \"${package_name}\"/name = \"${package_name}${PACKAGE_CUDA_SUFFIX}\"/g" ${pyproject_file} | ||
| echo "${version}" > VERSION | ||
| echo "${version_cpp}" > VERSION_CPP | ||
| echo "${version_cpp}" > "${package_dir}/VERSION" | ||
| sed -i "/^__git_commit__/ s/= .*/= \"${commit}\"/g" "${package_src_dir}/_version.py" | ||
|
|
||
| if [[ ${PACKAGE_CUDA_SUFFIX} == "-cu12" ]]; then | ||
| # change pyproject.toml to use CUDA 12.x version of cupy | ||
| sed -i "s/cupy-cuda11x/cupy-cuda12x/g" ${pyproject_file} | ||
| fi | ||
|
|
||
| echo "The package name and/or version was modified in the package source. The git diff is:" | ||
| git diff | ||
|
There was a problem hiding this comment. note: JFYI we've removed this from every other repo at this point but you're welcome to keep it if you find it useful for debugging. It's been a long time since we got much use out of it. There was a problem hiding this comment. I have now removed it |
||
|
|
||
| pip install --upgrade pip | ||
|
There was a problem hiding this comment. suggestion (blocking): This shouldn't be necessary. Were you running into specific problems? If so, do we need to address them somewhere? There was a problem hiding this comment. No, I was originally also installing a newer CMake etc here and was updating pip before doing that. I have now removed this line. |
||
|
|
||
| # Install pip build dependencies (not yet using pyproject.toml) | ||
| rapids-dependency-file-generator \ | ||
| --file_key "py_build" \ | ||
| --output "requirements" \ | ||
| --matrix "cuda=${RAPIDS_CUDA_VERSION%.*};arch=$(arch);py=${RAPIDS_PY_VERSION}" | tee build_requirements.txt | ||
| pip install -r build_requirements.txt | ||
|
Comment on lines
+34
to
+39
There was a problem hiding this comment. suggestion (blocking): Is this still true? Now that #617 is merged you should be using pyproject.toml. That said, I don't know how the C++ build works when you do There was a problem hiding this comment. Yes, I think it still needs to be this way as from the I agree that this is a bit non-standard and we can potentially look into moving to a scikit-build based approach in the future. I don't have time to focus on that at the moment, though. One benefit of the approach as it is now is that even Windows users can build the pure Python package using pip without having to have CMake and compile the C++ library (which supports linux only). They would be able to use all of There was a problem hiding this comment. I get that there's a separate There was a problem hiding this comment. Here's what I see on CI: # To make changes, edit dependencies.yaml and run `rapids-dependency-file-generator`.
cmake>=3.23.1,!=3.25.0
ninja
setuptools>=24.2.0
wheelThere was a problem hiding this comment. Trying deleting these lines in PR: #628 |
||
|
|
||
|
|
||
| # First build the C++ lib using CMake via the run script | ||
| echo "libcucim version: `cat VERSION_CPP`" | ||
| ./run build_local all ${CMAKE_BUILD_TYPE} | ||
|
|
||
| cd "${package_dir}" | ||
|
|
||
| python -m pip wheel . -w dist -vvv --no-deps --disable-pip-version-check | ||
|
|
||
| mkdir -p final_dist | ||
| python -m auditwheel repair -w final_dist dist/* | ||
|
|
||
| RAPIDS_PY_WHEEL_NAME="${package_name}_${RAPIDS_PY_CUDA_SUFFIX}" rapids-upload-wheels-to-s3 final_dist | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| #!/bin/bash | ||
| # Copyright (c) 2023, NVIDIA CORPORATION. | ||
|
|
||
| set -eou pipefail | ||
|
|
||
| RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen ${RAPIDS_CUDA_VERSION})" | ||
| RAPIDS_PY_WHEEL_NAME="cucim_${RAPIDS_PY_CUDA_SUFFIX}" rapids-download-wheels-from-s3 ./dist | ||
|
|
||
| # echo to expand wildcard before adding `[extra]` requires for pip | ||
| python -m pip install $(echo ./dist/cucim*.whl)[test] | ||
|
|
||
| # Run smoke tests for aarch64 pull requests | ||
| if [[ "$(arch)" == "aarch64" && ${RAPIDS_BUILD_TYPE} == "pull-request" ]]; then | ||
| python ./ci/wheel_smoke_test.py | ||
| else | ||
| # TODO: revisit enabling imagecodecs package during testing | ||
| python -m pytest ./python/cucim | ||
| fi |
grlee77 marked this conversation as resolved.
Show resolved
Hide resolved
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| import cupy as cp | ||
|
|
||
| import cucim | ||
| import cucim.skimage | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| # verify that all top-level modules are available | ||
| assert cucim.is_available("clara") | ||
| assert cucim.is_available("core") | ||
| assert cucim.is_available("skimage") | ||
|
|
||
| # generate a synthetic image and apply a filter | ||
| img = cucim.skimage.data.binary_blobs(length=512, n_dim=2) | ||
| assert isinstance(img, cp.ndarray) | ||
| assert img.dtype.kind == "b" | ||
| assert img.shape == (512, 512) | ||
|
|
||
| eroded = cucim.skimage.morphology.binary_erosion( | ||
| img, cp.ones((3, 3), dtype=bool) | ||
| ) |
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.
suggestion (non-blocking): The alternative would be maintaining a single file and then stripping out the alpha in CMake. Now that it's done this way I don't know if it's worth changing, but it would let you carry around one less file.
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.
Agreed. I did it this way because I am not familiar with the proper CMake syntax to do it. Happy to update it if you have suggestions on how the CMake code would look.
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.
This would probably do it (after the existing
file(STRING...)command):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.
Thanks, @vyasr, that seems to work! I pushed a commit to remove VERSION_CPP as well as simplify the CI scripts a bit (there was still writing of some unused VERSION file).
There should now be only a single top-level VERSION file (as well as symlink to it from
python/cucim/src/cucim/VERSION)