Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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 @@ -188,7 +188,7 @@ jobs:
package-type: python
matrix_filter: map(select((.CUDA_VER | startswith("12"))))
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
26 changes: 26 additions & 0 deletions ci/test_wheel_cuopt.sh
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,29 @@ 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

rapids-logger "building 'cvxpy' from source"
git clone -b cuopt_solver https://github.com/tmckayus/cvxpy
pushd ./cvxpy || exit 1
python -m build \
--wheel \
--outdir dist \
.

# NOTE: installing cvxpy[CUOPT] alongside CI artifacts is helpful to catch dependency conflicts
rapids-logger "installing 'cvxpy' with cuopt"
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]"

rapids-logger "running 'cvxpy' tests"
python -m pytest \
--error-for-skips \
-k "TestCUOPT" \
./cvxpy/tests/test_conic_solvers.py
Loading