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.

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

Fix CI workflows for pandas-tests and add test summary. #14847

Merged
merged 31 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
c6c2b78
Generate test summaries from wheel test jobs.
bdice Jan 23, 2024
b9ef194
Add RAPIDS_TESTS_DIR.
bdice Jan 24, 2024
2c576bf
Fix junit file names, add RESULTS_DIR to dask-cudf.
bdice Jan 24, 2024
de86653
Merge branch 'branch-24.04' into wheel-test-summary
bdice Jan 26, 2024
b78c27c
Revert some changes to pytest flags.
bdice Jan 26, 2024
67b1430
Use wheel-test-summary on all jobs.
bdice Jan 26, 2024
4d3afba
Hide test failures.
bdice Feb 1, 2024
c00b271
Merge remote-tracking branch 'upstream/branch-24.04' into wheel-test-…
bdice Feb 1, 2024
c743838
Update for pandas 2: --skip-slow is changed to -m "not slow".
bdice Feb 4, 2024
79baa49
Merge remote-tracking branch 'upstream/branch-24.04' into wheel-test-…
bdice Feb 4, 2024
cb09742
Fail CI if a command fails.
bdice Feb 4, 2024
9567fed
Remove unbound variable.
bdice Feb 4, 2024
a5c7eb1
Also fail in run-pandas-tests.sh if something errors.
bdice Feb 4, 2024
7075302
Merge branch 'branch-24.04' into wheel-test-summary
bdice Feb 5, 2024
3d5260f
Remove import mode
vyasr Feb 5, 2024
5088481
Remove from other script too
vyasr Feb 6, 2024
b8fba57
Also try removing durations
vyasr Feb 6, 2024
67a303c
Merge remote-tracking branch 'upstream/branch-24.04' into wheel-test-…
bdice Feb 7, 2024
7b3b361
Double quote
shwina Feb 7, 2024
9c6810e
Enter tests directories to avoid benchmark collection.
bdice Feb 7, 2024
fe0b29a
Add missing slash.
bdice Feb 7, 2024
16ac70f
Revert to branch-24.04.
bdice Feb 7, 2024
c8f0290
Ignore tests/interchange/test_impl.py.
bdice Feb 7, 2024
f6688f9
Merge branch 'wheel-test-summary' of github.com:bdice/cudf into wheel…
bdice Feb 7, 2024
d71a0de
Merge remote-tracking branch 'upstream/branch-24.04' into wheel-test-…
bdice Feb 8, 2024
7f12d1a
Permit test failures.
bdice Feb 8, 2024
fa7b996
Add back removed flags.
bdice Feb 8, 2024
f062e28
Simplify optional handling of RAPIDS_REF_NAME.
bdice Feb 8, 2024
47fb2f0
Merge branch 'branch-24.04' into wheel-test-summary
bdice Feb 8, 2024
9b3c322
cd into tests, to avoid finding "pandas" module in cudf.
bdice Feb 8, 2024
264b3cf
Merge branch 'wheel-test-summary' of github.com:bdice/cudf into wheel…
bdice Feb 8, 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
2 changes: 2 additions & 0 deletions .github/workflows/pr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,8 @@ jobs:
matrix_filter: map(select(.ARCH == "amd64")) | max_by(.CUDA_VER) | [.]
build_type: pull-request
script: ci/cudf_pandas_scripts/pandas-tests/run.sh pr
# Hide test failures because they exceed the GITHUB_STEP_SUMMARY output limit.
test_summary_show: "none"
#pandas-tests-diff:
# # diff the results of running the Pandas unit tests and publish a job summary
# needs: [pandas-tests-main, pandas-tests-pr]
Expand Down
14 changes: 9 additions & 5 deletions ci/cudf_pandas_scripts/pandas-tests/run.sh
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
#!/usr/bin/env bash
# SPDX-FileCopyrightText: Copyright (c) 2023 NVIDIA CORPORATION & AFFILIATES.
# SPDX-FileCopyrightText: Copyright (c) 2023-2024, NVIDIA CORPORATION & AFFILIATES.
# All rights reserved.
# SPDX-License-Identifier: Apache-2.0

set -euo pipefail

PANDAS_TESTS_BRANCH=${1}

rapids-logger "Running Pandas tests using $PANDAS_TESTS_BRANCH branch"
rapids-logger "PR number: $RAPIDS_REF_NAME"
rapids-logger "PR number: ${RAPIDS_REF_NAME:-"unknown"}"
vyasr marked this conversation as resolved.
Show resolved Hide resolved

# Set the manylinux version used for downloading the wheels so that we test the
# newer ABI wheels on the newer images that support their installation.
Expand All @@ -25,14 +27,16 @@ RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen ${RAPIDS_CUDA_VERSION})"
RAPIDS_PY_WHEEL_NAME="cudf_${manylinux}_${RAPIDS_PY_CUDA_SUFFIX}" rapids-download-wheels-from-s3 ./local-cudf-dep
python -m pip install $(ls ./local-cudf-dep/cudf*.whl)[test,pandas-tests]

git checkout $COMMIT
RESULTS_DIR=${RAPIDS_TESTS_DIR:-"$(mktemp -d)"}
RAPIDS_TESTS_DIR=${RAPIDS_TESTS_DIR:-"${RESULTS_DIR}/test-results"}/
mkdir -p "${RAPIDS_TESTS_DIR}"

bash python/cudf/cudf/pandas/scripts/run-pandas-tests.sh \
-n 10 \
--tb=line \
--skip-slow \
-m "not slow" \
--max-worker-restart=3 \
--import-mode=importlib \
--junitxml="${RAPIDS_TESTS_DIR}/junit-cudf-pandas.xml" \
--report-log=${PANDAS_TESTS_BRANCH}.json 2>&1

# summarize the results and save them to artifacts:
Expand Down
17 changes: 15 additions & 2 deletions ci/test_wheel_cudf.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#!/bin/bash
# Copyright (c) 2023, NVIDIA CORPORATION.
# Copyright (c) 2023-2024, NVIDIA CORPORATION.

set -eou pipefail

Expand All @@ -22,9 +22,22 @@ RAPIDS_PY_WHEEL_NAME="cudf_${manylinux}_${RAPIDS_PY_CUDA_SUFFIX}" rapids-downloa
# echo to expand wildcard before adding `[extra]` requires for pip
python -m pip install $(echo ./dist/cudf*.whl)[test]

RESULTS_DIR=${RAPIDS_TESTS_DIR:-"$(mktemp -d)"}
RAPIDS_TESTS_DIR=${RAPIDS_TESTS_DIR:-"${RESULTS_DIR}/test-results"}/
mkdir -p "${RAPIDS_TESTS_DIR}"

# Run smoke tests for aarch64 pull requests
if [[ "$(arch)" == "aarch64" && ${RAPIDS_BUILD_TYPE} == "pull-request" ]]; then
rapids-logger "Run smoke tests for cudf"
python ./ci/wheel_smoke_test_cudf.py
else
python -m pytest -n 8 ./python/cudf/cudf/tests
rapids-logger "pytest cudf"
pushd python/cudf/cudf
python -m pytest \
--cache-clear \
--junitxml="${RAPIDS_TESTS_DIR}/junit-cudf.xml" \
--numprocesses=8 \
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you choose to use --numprocesses instead of -n to match the conda tests? I'm fine with either option but I think (without having done any real investigation) that -n is more common. That may also be a dichotomy between wheels/conda (since we implemented GHA for each in parallel at roughly the same time there was less coordination/enforcement of uniformity) that we could try to unify over time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We've generally chosen to write the full names of flags wherever possible in our CI scripts. You're correct, this was an artifact of us doing isolated work on conda and wheels at the same time. I took this opportunity to make them match.

--dist=loadscope \
./tests
popd
fi
14 changes: 12 additions & 2 deletions ci/test_wheel_dask_cudf.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#!/bin/bash
# Copyright (c) 2023, NVIDIA CORPORATION.
# Copyright (c) 2023-2024, NVIDIA CORPORATION.

set -eou pipefail

Expand All @@ -26,5 +26,15 @@ python -m pip install --no-deps ./local-cudf-dep/cudf*.whl
# echo to expand wildcard before adding `[extra]` requires for pip
python -m pip install $(echo ./dist/dask_cudf*.whl)[test]

RESULTS_DIR=${RAPIDS_TESTS_DIR:-"$(mktemp -d)"}
RAPIDS_TESTS_DIR=${RAPIDS_TESTS_DIR:-"${RESULTS_DIR}/test-results"}/
mkdir -p "${RAPIDS_TESTS_DIR}"

# Run tests in dask_cudf/tests and dask_cudf/io/tests
python -m pytest -n 8 ./python/dask_cudf/dask_cudf/
rapids-logger "pytest dask_cudf"
pushd python/dask_cudf/dask_cudf
python -m pytest \
--junitxml="${RAPIDS_TESTS_DIR}/junit-dask-cudf.xml" \
--numprocesses=8 \
.
popd
10 changes: 6 additions & 4 deletions python/cudf/cudf/pandas/scripts/run-pandas-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@
#
# This script creates a `pandas-testing` directory if it doesn't exist

set -euo pipefail

# Grab the Pandas source corresponding to the version
# of Pandas installed.
PANDAS_VERSION=$(python -c "import pandas; print(pandas.__version__)")

PYTEST_IGNORES="--ignore=tests/io/test_user_agent.py"
PYTEST_IGNORES="--ignore=tests/io/test_user_agent.py --ignore=tests/interchange/test_impl.py"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just inline this variable? It's only used once, it's defined far from that location (the pytest call), and it's incongruous with how we provide basically every other argument inline.

Copy link
Contributor Author

@bdice bdice Feb 8, 2024

Choose a reason for hiding this comment

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

It's "far" in lines of code but not really that far semantically. It's just that there's another very large variable definition in between. I think I'm going to leave this as-is because I don't think it is worth another CI run to change.


mkdir -p pandas-testing
cd pandas-testing
Expand Down Expand Up @@ -92,7 +93,7 @@ cd pandas-tests/
# test_overwrite_warns unsafely patchs over Series.mean affecting other tests when run in parallel
# test_complex_series_frame_alignment randomly selects a DataFrames and axis to test but particular random selection(s) always fails
# test_numpy_ufuncs_basic compares floating point values to unbounded precision, sometimes leading to failures
TEST_NUMPY_UFUNCS_BASIC_FLAKY="test_numpy_ufuncs_basic[float-exp] \
TEST_NUMPY_UFUNCS_BASIC_FLAKY="not test_numpy_ufuncs_basic[float-exp] \
and not test_numpy_ufuncs_basic[float-exp2] \
and not test_numpy_ufuncs_basic[float-expm1] \
and not test_numpy_ufuncs_basic[float-log] \
Expand Down Expand Up @@ -183,11 +184,12 @@ and not test_numpy_ufuncs_basic[nullable_float-rad2deg]"

PANDAS_CI="1" python -m pytest -p cudf.pandas \
-m "not single_cpu and not db" \
-k "not test_overwrite_warns and not test_complex_series_frame_alignment and not $TEST_NUMPY_UFUNCS_BASIC_FLAKY" \
-k "not test_overwrite_warns and not test_complex_series_frame_alignment and $TEST_NUMPY_UFUNCS_BASIC_FLAKY" \
--durations=50 \
--import-mode=importlib \
bdice marked this conversation as resolved.
Show resolved Hide resolved
-o xfail_strict=True \
${PYTEST_IGNORES} $@
${PYTEST_IGNORES} \
"$@" || [ $? = 1 ] # Exit success if exit code was 1 (permit test failures but not other errors)

mv *.json ..
cd ..
Expand Down
Loading