Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
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
2 changes: 1 addition & 1 deletion .github/workflows/pr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ jobs:
package-name: cuopt
package-type: python
wheel-tests-cuopt:
needs: [wheel-build-cuopt, wheel-build-cuopt-mps-parser, changed-files]
needs: [wheel-build-cuopt, wheel-build-cuopt-mps-parser, wheel-build-cuopt-sh-client, changed-files]
secrets: inherit
uses: rapidsai/shared-workflows/.github/workflows/wheels-test.yaml@branch-25.08
#if: fromJSON(needs.changed-files.outputs.changed_file_groups).test_python_cuopt
Expand Down
16 changes: 16 additions & 0 deletions ci/test_wheel_cuopt.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,33 @@

set -euo pipefail

# sets up a constraints file for 'pip' and puts its location in an exported variable PIP_EXPORT,
# so those constraints will affect all future 'pip install' calls
source rapids-init-pip

# Download the packages built in the previous step
RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen "${RAPIDS_CUDA_VERSION}")"
CUOPT_MPS_PARSER_WHEELHOUSE=$(RAPIDS_PY_WHEEL_NAME="cuopt_mps_parser" rapids-download-wheels-from-github python)
CUOPT_SH_CLIENT_WHEELHOUSE=$(RAPIDS_PY_WHEEL_NAME="cuopt_sh_client" rapids-download-wheels-from-github python)
CUOPT_WHEELHOUSE=$(RAPIDS_PY_WHEEL_NAME="cuopt_${RAPIDS_PY_CUDA_SUFFIX}" rapids-download-wheels-from-github python)
LIBCUOPT_WHEELHOUSE=$(RAPIDS_PY_WHEEL_NAME="libcuopt_${RAPIDS_PY_CUDA_SUFFIX}" rapids-download-wheels-from-github cpp)

# update pip constraints.txt to ensure all future 'pip install' (including those in ci/thirdparty-testing)
# use these wheels for cuopt packages
cat > "${PIP_CONSTRAINT}" <<EOF
cuopt-${RAPIDS_PY_CUDA_SUFFIX} @ file://$(echo ${CUOPT_WHEELHOUSE}/cuopt_${RAPIDS_PY_CUDA_SUFFIX}-*.whl)
cuopt-mps-parser @ file://$(echo ${CUOPT_MPS_PARSER_WHEELHOUSE}/cuopt_mps_parser-*.whl)
cuopt-sh-client @ file://$(echo ${CUOPT_SH_CLIENT_WHEELHOUSE}/cuopt_sh_client-*.whl)
libcuopt-${RAPIDS_PY_CUDA_SUFFIX} @ file://$(echo ${LIBCUOPT_WHEELHOUSE}/libcuopt_${RAPIDS_PY_CUDA_SUFFIX}-*.whl)
EOF

# echo to expand wildcard before adding `[extra]` requires for pip
rapids-pip-retry install \
--extra-index-url=https://pypi.nvidia.com \
--constraint "${PIP_CONSTRAINT}" \
"${CUOPT_MPS_PARSER_WHEELHOUSE}"/cuopt_mps_parser*.whl \
"$(echo "${CUOPT_WHEELHOUSE}"/cuopt*.whl)[test]" \
"${CUOPT_SH_CLIENT_WHEELHOUSE}"/cuopt_sh_client*.whl \
"${LIBCUOPT_WHEELHOUSE}"/libcuopt*.whl

python -c "import cuopt"
Expand Down Expand Up @@ -59,3 +73,5 @@ timeout 10m bash ./python/libcuopt/libcuopt/tests/test_cli.sh
# Run Python tests
RAPIDS_DATASET_ROOT_DIR=./datasets timeout 30m python -m pytest --verbose --capture=no ./python/cuopt/cuopt/tests/

# run cvxpy integration tests
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jameslamb We would have more integration tests in future, should we move few items to be partly into a bash script and run it from there to keep the test_wheel_cuopt.sh simple ?

Copy link
Member

Choose a reason for hiding this comment

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

Sure we can, and to be fair that's how @tmckayus had it originally before I pushed commits here.

Let me tell you why I did it this way. I was mainly concerned about making sure that the pip install that installs cvxpy uses the just-built-in-this-CI-run cuopt packages.

Doing it in 1 script like this makes that easy to guarantee, because I can just reference variables from the same scope and packages downloaded a few lines higher up. Like this:

CUOPT_SH_CLIENT_WHEELHOUSE=$(RAPIDS_PY_WHEEL_NAME="cuopt_sh_client" rapids-download-wheels-from-github python)
python -m pip install \
    --extra-index-url=https://pypi.nvidia.com \
    "${CUOPT_MPS_PARSER_WHEELHOUSE}"/cuopt_mps_parser*.whl \
    "${CUOPT_SH_CLIENT_WHEELHOUSE}"/cuopt_sh_client*.whl \
    "${CUOPT_WHEELHOUSE}"/cuopt*.whl \
    "${LIBCUOPT_WHEELHOUSE}"/libcuopt*.whl \
    'pytest-error-for-skips>=2.0.2' \
    "$(echo ./dist/cvxpy*.whl)[CUOPT,testing]"

If you want to defer all the cvxpy stuff to a separate script, like Trevor had it before, it'd involve some changes to how packages are downloaded.

I can do that here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we might be adding few more smoke tests to this, I felt the script might become too huge and it might not be scaled. Yeah, lets move these to a folder "thirdparty-testing" and add a script there to trigger test. Please let me know if your hands are full, I can work on this.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I will push changes here.

Copy link
Member

Choose a reason for hiding this comment

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

I just pushed changes moving cvxpy tests to their own file, as @tmckayus had originally (but using the name thirdparty-testing/ you asked for).

Hopefully this will set up a pattern that can be reliably replicated when other integration tests like this are added.

I will check back in an hour or so to be sure this worked. I especially want to see evidence in the logs that the just-built-in-CI cuopt, cuopt-mps-parser, etc. wheels were used, not packages downloaded from the nightly index.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you @jameslamb

./ci/thirdparty-testing/run_cvxpy_tests.sh
44 changes: 44 additions & 0 deletions ci/thirdparty-testing/run_cvxpy_tests.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
#!/bin/bash
# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
# SPDX-License-Identifier: Apache-2.0
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

set -e -u -o pipefail

echo "building 'cvxpy' from source"
git clone https://github.com/cvxpy/cvxpy.git
pushd ./cvxpy || exit 1
pip wheel \
-w dist \
.

# NOTE: installing cvxpy[CUOPT] alongside CI artifacts is helpful to catch dependency conflicts
echo "installing 'cvxpy' with cuopt"
python -m pip install \
--constraint "${PIP_CONSTRAINT}" \
--extra-index-url=https://pypi.nvidia.com \
--extra-index-url=https://pypi.anaconda.org/rapidsai-wheels-nightly/simple \
'pytest-error-for-skips>=2.0.2' \
"$(echo ./dist/cvxpy*.whl)[CUOPT,testing]"

# ensure that environment is still consistent (i.e. cvxpy requirements do not conflict with cuopt's)
pip check

echo "running 'cvxpy' tests"
timeout 3m python -m pytest \
--verbose \
--capture=no \
--error-for-skips \
-k "TestCUOPT" \
./cvxpy/tests/test_conic_solvers.py