Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
4 changes: 4 additions & 0 deletions .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ jobs:
script: ci/build_wheel_cuopt.sh
package-name: cuopt
package-type: python
# abi3 wheel: build one per ARCH + CUDA major, using the minimum supported Python.
# Grouping on the CUDA major (not the full version) keeps one job per published
# artifact name, which only encodes the major.
matrix_filter: group_by([.ARCH, (.CUDA_VER|split(".")|map(tonumber)|.[0])]) | map(min_by((.PY_VER|split(".")|map(tonumber)), (.CUDA_VER|split(".")|map(-tonumber))))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see this came from CodeRabbit's suggestion you posted:

Matrix grouping (valid, fixed). The filter grouped on the full CUDA_VER, but artifact names encode only the CUDA major. With two minors in one major that would have produced two jobs writing the same artifact name. Now grouped on [.ARCH, CUDA major] with a deterministic minor tiebreak, matching the existing libcuopt filter. Checked against both matrices — identical output today (4 jobs, all 3.11), 8 → 4 on a matrix with two minors per major.

This is fine but it's unnecessary complexity. The matrix we use for wheel builds very intentionally only chooses a single minor version from within each CUDA major family.

I cannot foresee a situation where that matrix would have multiple minor versions from the same major version in it.

I have a weak preference for reverting this back to the form used in cudf and all other RAPIDS repos:

https://github.com/NVIDIA/cudf/blob/a98ad619fea197d1d0bfa0c2ed04ba50bd084ec4/.github/workflows/pr.yaml#L765

Consistency makes automation easier.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Makes sense — reverted in b9165cc. Both pr.yaml and build.yaml now use the exact form you linked, comment included:

# Build a wheel for each CUDA x ARCH x minimum supported Python version
matrix_filter: group_by({CUDA_VER, ARCH}) | map(min_by(.PY_VER | split(".") | map(tonumber)))

Good to know the wheel matrix guarantees one minor per CUDA major — that was the assumption I couldn't confirm from outside, so I'd hedged against it.

While you're here: there's one more deliberate divergence I'd like your call on, in ci/validate_wheel.sh. CodeRabbit flagged that the abi3audit step silently no-ops when find matches nothing, so I tightened the glob to *-abi3-*.whl and added a failure when python/cuopt produces no abi3 wheel — the idea being that dropping wheel.py-api would otherwise skip the audit and leave CI green. It's scoped to python/cuopt because libcuopt/server/sh-client share the script and are all py3-none.

By the same consistency argument you just made, I'm happy to drop it and match cudf/rmm exactly. Your call — it's a one-line revert either way.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I intentionally left out cases like if [[ "${package_dir}" == "python/cuopt" ]], because I didn't want anyone to have to remember to update that code as the set of packages changes (which could happen, similar to #1622).

dropping wheel.py-api would otherwise skip the audit and leave CI green

This would already be caught by other things in CI. If you omitted that, the wheel would be a cp311-cp311-manylinux wheel (because it was built on Python 3.11 thanks to the matrix_filter), and Python 3.12+ wheel-testing CI jobs should fail with a loud error about not being able to find artifacts / install packages.

IMO it should be reverted back to the form I used in the other PRs. But I don't feel strongly enough to block the PR over it.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Reverted in 187f06aci/validate_wheel.sh now matches cudf's version byte for byte (diffed to confirm).

You're right that the case is already covered, and I checked it rather than take my own word for it: wheel-tests-cuopt runs on 3.11, 3.12, 3.13 and 3.14, so a cp311-cp311 wheel would fail to install in 6 of the 8 test jobs. Loud enough, and it doesn't depend on anyone maintaining a package list.

The point about #1622 is the one that actually changes how I'd approach this next time — I was optimizing for catching a regression and hadn't weighed the cost of a conditional that silently goes stale when the package set moves. Thanks for spelling it out.

wheel-publish-cuopt:
needs: wheel-build-cuopt
permissions:
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/pr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ jobs:
runs-on: ubuntu-latest
outputs:
libcuopt_filter: ${{ steps.set-filters.outputs.libcuopt_filter }}
cuopt_filter: ${{ steps.set-filters.outputs.cuopt_filter }}
cuopt_server_filter: ${{ steps.set-filters.outputs.cuopt_server_filter }}
cuopt_server_test_filter: ${{ steps.set-filters.outputs.cuopt_server_test_filter }}
cuopt_sh_client_filter: ${{ steps.set-filters.outputs.cuopt_sh_client_filter }}
Expand All @@ -56,6 +57,7 @@ jobs:
id: set-filters
run: |
echo "libcuopt_filter=group_by([.ARCH, (.CUDA_VER|split(\".\")|map(tonumber)|.[0])]) | map(max_by(.PY_VER|split(\".\")|map(tonumber)))" >> $GITHUB_OUTPUT
echo "cuopt_filter=group_by([.ARCH, (.CUDA_VER|split(\".\")|map(tonumber)|.[0])]) | map(min_by((.PY_VER|split(\".\")|map(tonumber)), (.CUDA_VER|split(\".\")|map(-tonumber))))" >> $GITHUB_OUTPUT
echo "cuopt_server_filter=map(select(.ARCH == \"amd64\")) | group_by(.CUDA_VER|split(\".\")|map(tonumber)|.[0]) | map(max_by([(.PY_VER|split(\".\")|map(tonumber)), (.CUDA_VER|split(\".\")|map(tonumber))]))" >> $GITHUB_OUTPUT
echo "cuopt_server_test_filter=map(select(.ARCH == \"amd64\")) | group_by(.CUDA_VER | split(\".\") | map(tonumber) | .[0]) | map(max_by([(.PY_VER | split(\".\") | map(tonumber)), (.CUDA_VER | split(\".\") | map(tonumber))]))" >> $GITHUB_OUTPUT
echo "cuopt_sh_client_filter=[map(select(.ARCH == \"amd64\")) | min_by((.PY_VER | split(\".\") | map(tonumber)), (.CUDA_VER | split(\".\") | map(-tonumber)))]" >> $GITHUB_OUTPUT
Expand Down Expand Up @@ -479,6 +481,8 @@ jobs:
script: ci/build_wheel_cuopt.sh
package-name: cuopt
package-type: python
# abi3 wheel: build one per ARCH + CUDA major, using the minimum supported Python
matrix_filter: ${{ needs.compute-matrix-filters.outputs.cuopt_filter }}
wheel-tests-cuopt:
needs: [wheel-build-cuopt, wheel-build-cuopt-sh-client, changed-files]
permissions:
Expand Down
3 changes: 3 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,9 @@ To build all libraries and tests, simply run
```

- **Note**: if Cython files (`*.pyx` or `*.pxd`) have changed, the Python build must be rerun.
- **Note**: the `cuopt` wheel is built against the CPython Limited API (abi3), so Cython code must
use only APIs the Limited API exposes. An unsupported API fails to compile under
`-DPy_LIMITED_API` rather than failing at runtime.

To run the C++ tests, run

Expand Down
21 changes: 21 additions & 0 deletions ci/build_wheel.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,22 @@ set -euo pipefail

package_name=$1
package_dir=$2
shift 2

# Parse optional flags
stable_abi=false
while [[ $# -gt 0 ]]; do
case "$1" in
--stable)
stable_abi=true
shift
;;
*)
echo "Unknown option: $1" >&2
exit 1
;;
esac
done

source rapids-configure-sccache
source rapids-datetime-string
Expand All @@ -29,6 +45,11 @@ RAPIDS_PIP_WHEEL_ARGS=(
--disable-pip-version-check
)

# Add py-api setting for stable ABI builds
if [[ "${stable_abi}" == "true" ]] && [[ -n "${RAPIDS_PY_API:-}" ]]; then
RAPIDS_PIP_WHEEL_ARGS+=(--config-settings="skbuild.wheel.py-api=${RAPIDS_PY_API}")
fi

# Only use --build-constraint when build isolation is enabled.
#
# Passing '--build-constraint' and '--no-build-isolation` together results in an error from 'pip',
Expand Down
9 changes: 7 additions & 2 deletions ci/build_wheel_cuopt.sh
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,17 @@ EXCLUDE_ARGS=(
--exclude "librmm.so"
)

ci/build_wheel.sh cuopt ${package_dir}
# TODO: move this variable into `ci-wheel`
# Format Python limited API version string
RAPIDS_PY_API="cp${RAPIDS_PY_VERSION//./}"
export RAPIDS_PY_API

ci/build_wheel.sh cuopt ${package_dir} --stable

# repair wheels and write to the location that artifact-uploading code expects to find them
python -m auditwheel repair "${EXCLUDE_ARGS[@]}" -w ${RAPIDS_WHEEL_BLD_OUTPUT_DIR} ${package_dir}/dist/*

ci/validate_wheel.sh "${package_dir}" "${RAPIDS_WHEEL_BLD_OUTPUT_DIR}"

RAPIDS_PACKAGE_NAME="$(rapids-artifact-name wheel_python cuopt cuopt --py "$RAPIDS_PY_VERSION" --cuda "$RAPIDS_CUDA_VERSION")"
RAPIDS_PACKAGE_NAME="$(rapids-artifact-name wheel_python cuopt cuopt --stable --cuda "$RAPIDS_CUDA_VERSION")"
export RAPIDS_PACKAGE_NAME
2 changes: 1 addition & 1 deletion ci/test_self_hosted_service.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ source rapids-init-pip

# Download the cuopt built in the previous step
LIBCUOPT_WHEELHOUSE=$(rapids-download-from-github "$(rapids-artifact-name wheel_cpp libcuopt cuopt --cuda "$RAPIDS_CUDA_VERSION")")
CUOPT_WHEELHOUSE=$(rapids-download-from-github "$(rapids-artifact-name wheel_python cuopt cuopt --py "$RAPIDS_PY_VERSION" --cuda "$RAPIDS_CUDA_VERSION")")
CUOPT_WHEELHOUSE=$(rapids-download-from-github "$(rapids-artifact-name wheel_python cuopt cuopt --stable --cuda "$RAPIDS_CUDA_VERSION")")
CUOPT_SERVER_WHEELHOUSE=$(rapids-download-from-github "$(rapids-artifact-name wheel_python cuopt-server cuopt --pure --arch any --cuda "$RAPIDS_CUDA_VERSION")")

# generate constraints (possibly pinning to oldest support versions of dependencies)
Expand Down
2 changes: 1 addition & 1 deletion ci/test_wheel_cuopt.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ bash "$(dirname "$(realpath "${BASH_SOURCE[0]}")")/utils/install_openssl3_runtim
# Download the packages built in the previous step
RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen "${RAPIDS_CUDA_VERSION}")"
LIBCUOPT_WHEELHOUSE=$(rapids-download-from-github "$(rapids-artifact-name wheel_cpp libcuopt cuopt --cuda "$RAPIDS_CUDA_VERSION")")
CUOPT_WHEELHOUSE=$(rapids-download-from-github "$(rapids-artifact-name wheel_python cuopt cuopt --py "$RAPIDS_PY_VERSION" --cuda "$RAPIDS_CUDA_VERSION")")
CUOPT_WHEELHOUSE=$(rapids-download-from-github "$(rapids-artifact-name wheel_python cuopt cuopt --stable --cuda "$RAPIDS_CUDA_VERSION")")
CUOPT_SH_CLIENT_WHEELHOUSE=$(rapids-download-from-github "$(rapids-artifact-name wheel_python cuopt-sh-client cuopt --pure --arch any)")

# update pip constraints.txt to ensure all future 'pip install' (including those in ci/thirdparty-testing)
Expand Down
2 changes: 1 addition & 1 deletion ci/test_wheel_cuopt_server.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ bash "$(dirname "$(realpath "${BASH_SOURCE[0]}")")/utils/install_openssl3_runtim

# Download the packages built in the previous step
LIBCUOPT_WHEELHOUSE=$(rapids-download-from-github "$(rapids-artifact-name wheel_cpp libcuopt cuopt --cuda "$RAPIDS_CUDA_VERSION")")
CUOPT_WHEELHOUSE=$(rapids-download-from-github "$(rapids-artifact-name wheel_python cuopt cuopt --py "$RAPIDS_PY_VERSION" --cuda "$RAPIDS_CUDA_VERSION")")
CUOPT_WHEELHOUSE=$(rapids-download-from-github "$(rapids-artifact-name wheel_python cuopt cuopt --stable --cuda "$RAPIDS_CUDA_VERSION")")
CUOPT_SERVER_WHEELHOUSE=$(rapids-download-from-github "$(rapids-artifact-name wheel_python cuopt-server cuopt --pure --arch any --cuda "$RAPIDS_CUDA_VERSION")")
CUOPT_SH_CLIENT_WHEELHOUSE=$(rapids-download-from-github "$(rapids-artifact-name wheel_python cuopt-sh-client cuopt --pure --arch any)")

Expand Down
21 changes: 21 additions & 0 deletions ci/validate_wheel.sh
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,24 @@ rapids-logger "validate packages with 'twine'"
twine check \
--strict \
"$(echo "${wheel_dir_relative_path}"/*.whl)"

rapids-logger "validate packages with 'abi3audit'"

# 'abi3audit' fails on wheels with DSOs that lack an ABI tag, so only the abi3 wheels
# are audited. Of the packages sharing this script, only 'cuopt' builds one; the rest
# are 'py3-none'.
abi3_wheels=()
while IFS= read -r -d '' wheel; do
abi3_wheels+=("${wheel}")
done < <(find "${wheel_dir_relative_path}" -type f -name '*-abi3-*.whl' -print0)

# Guard against 'cuopt' silently losing its abi3 tag: without this, dropping
# 'wheel.py-api' would skip the audit entirely and leave CI green.
if [[ "${package_dir}" == "python/cuopt" ]] && [[ "${#abi3_wheels[@]}" -eq 0 ]]; then
rapids-echo-stderr "expected an abi3 wheel in '${wheel_dir_relative_path}', found none"
exit 1
fi

if [[ "${#abi3_wheels[@]}" -gt 0 ]]; then
abi3audit --strict --summary --verbose "${abi3_wheels[@]}"
fi
2 changes: 1 addition & 1 deletion conda/environments/all_cuda-129_arch-aarch64.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ dependencies:
- cudf==26.10.*,>=0.0.0a0
- cupy>=14.0.1,!=14.1.0
- cxx-compiler
- cython>=3.0.3
- cython>=3.2.2,<3.3.0a0
- docutils>=0.21
- doxygen=1.9.1
- fastapi
Expand Down
2 changes: 1 addition & 1 deletion conda/environments/all_cuda-129_arch-x86_64.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ dependencies:
- cudf==26.10.*,>=0.0.0a0
- cupy>=14.0.1,!=14.1.0
- cxx-compiler
- cython>=3.0.3
- cython>=3.2.2,<3.3.0a0
- docutils>=0.21
- doxygen=1.9.1
- fastapi
Expand Down
2 changes: 1 addition & 1 deletion conda/environments/all_cuda-133_arch-aarch64.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ dependencies:
- cudf==26.10.*,>=0.0.0a0
- cupy>=14.0.1,!=14.1.0
- cxx-compiler
- cython>=3.0.3
- cython>=3.2.2,<3.3.0a0
- docutils>=0.21
- doxygen=1.9.1
- fastapi
Expand Down
2 changes: 1 addition & 1 deletion conda/environments/all_cuda-133_arch-x86_64.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ dependencies:
- cudf==26.10.*,>=0.0.0a0
- cupy>=14.0.1,!=14.1.0
- cxx-compiler
- cython>=3.0.3
- cython>=3.2.2,<3.3.0a0
- docutils>=0.21
- doxygen=1.9.1
- fastapi
Expand Down
2 changes: 1 addition & 1 deletion dependencies.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ dependencies:
common:
- output_types: [conda, requirements, pyproject]
packages:
- cython>=3.0.3
- cython>=3.2.2,<3.3.0a0
test_python_common:
common:
- output_types: [conda, requirements, pyproject]
Expand Down
3 changes: 2 additions & 1 deletion python/cuopt/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ minimum-version = "build-system.requires"
ninja.make-fallback = false
sdist.reproducible = true
wheel.packages = ["cuopt"]
wheel.py-api = "cp311" # overridden in CI builds by arguments from `ci/build_wheel_cuopt.sh`

[tool.scikit-build.metadata.version]
provider = "scikit_build_core.metadata.regex"
Expand All @@ -102,7 +103,7 @@ matrix-entry = "cuda_suffixed=true;use_cuda_wheels=true"
requires = [
"cmake>=4.0",
"cupy-cuda13x[ctk]>=14.0.1,!=14.1.0",
"cython>=3.0.3",
"cython>=3.2.2,<3.3.0a0",
"libcuopt==26.10.*,>=0.0.0a0",
"ninja",
"pylibraft==26.10.*,>=0.0.0a0",
Expand Down
1 change: 1 addition & 0 deletions skills/cuopt-developer/references/contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ Follow YAGNI strictly here — flags, fallbacks, env-var overrides, and config k
A few non-YAGNI points worth keeping in mind:

- Prefer extending an existing script over adding a new one.
- For build/CI conventions shared across RAPIDS (wheel packaging, artifact naming, matrix filters), read the reference implementation in `rapidsai/rmm` or `NVIDIA/cudf` on `main` before writing anything. cuOpt's `ci/` scripts and shared-workflow inputs are near-copies of theirs, and gha-tools expects exact conventions — an equivalent-but-different local invention silently breaks the download side, which looks for the name the build side wrote.
- Validate inputs at the top, before any expensive work.
- One shell command per line over chained `&&`; no comments that restate the next line.
- Keep informational CI jobs (reporting, dashboards, comment posting) out of any required-checks list.
Expand Down
Loading