-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Shard pandas tests across runners in PR and nightly CI #22992
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
Merged
Merged
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
29d7a7c
fix
galipremsagar 0e72ffb
update
galipremsagar 950e580
Skip order-dependent tests failing in sharded pandas-tests CI
galipremsagar ddfacb4
Shard the nightly pandas-tests run too
galipremsagar 58ae23c
Harden the sharding configuration and summary
galipremsagar 29c49b2
Merge branch 'main' into split_pandas_tests
galipremsagar b08237e
Skip two tests failing in the sharded pandas run
galipremsagar 8a201dd
Merge branch 'main' into split_pandas_tests
galipremsagar 17177dc
Merge branch 'main' into split_pandas_tests
galipremsagar 8fa52ae
Address review feedback on the sharding change
galipremsagar 70df780
Merge branch 'main' into split_pandas_tests
galipremsagar b745302
[pre-commit.ci] auto code formatting
pre-commit-ci[bot] a9e4961
Drop secrets: inherit from the pandas-tests jobs
galipremsagar db69e7a
Merge branch 'main' into split_pandas_tests
galipremsagar a7391fe
Merge branch 'main' into split_pandas_tests
galipremsagar 72e088c
Add pandas-tests-shards to the pr-builder dependencies
galipremsagar 691447c
Address CodeRabbit review on the shard result handling
galipremsagar File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| #!/usr/bin/env bash | ||
| # SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| # Combine the results of the sharded nightly ("main") pandas-tests jobs. | ||
| # | ||
| # Each shard uploads its partial per-module summary as the GitHub artifact | ||
| # "pandas-test-main-results-<shard_id>". This job downloads them all and merges | ||
| # them into a single main-results.json, which is re-uploaded under that name so | ||
| # that PR runs keep finding it with `gh run download --name main-results.json`. | ||
| # | ||
| # Usage: | ||
| # merge-nightly.sh <num_shards> | ||
| # | ||
| # Unlike the PR-side summary.sh, this script is NOT best effort: main-results.json | ||
| # is the baseline every PR diffs against, and a partial or missing file would show | ||
| # up as spurious "new failures" in those PRs. Failing loudly instead keeps the run | ||
| # from being picked up as the latest successful nightly. | ||
|
|
||
| set -euo pipefail | ||
|
|
||
| source rapids-init-pip | ||
| # shellcheck source=ci/cudf_pandas_scripts/pandas-tests/shard-results.sh | ||
| source ci/cudf_pandas_scripts/pandas-tests/shard-results.sh | ||
|
|
||
| NUM_SHARDS=${1:?usage: merge-nightly.sh <num_shards>} | ||
|
|
||
| rapids-logger "Merging pandas-tests results from ${NUM_SHARDS} shards" | ||
|
|
||
| # set -e propagates a missing or unmergeable shard, which is what we want here: | ||
| # see the header comment. | ||
| merge_shard_results "pandas-test-main-results" "main-results.json" \ | ||
| "${NUM_SHARDS}" main-results.json | ||
|
|
||
| rapids-logger "Wrote main-results.json" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| # SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| """ | ||
| Merge the per-module summaries produced by ``summarize-test-results.py`` for | ||
| several test shards into a single summary. | ||
|
|
||
| Each shard runs a disjoint subset of the suite (see the ``--num-shards`` | ||
| sharding in ``pandas-testing-plugin.py``), so the combined result for a module | ||
| is obtained by summing every numeric field (test counts and the GPU/CPU | ||
| function-call counters) across the shards. The same module may appear in more | ||
| than one shard because sharding is per test, not per module. | ||
|
|
||
| Examples | ||
| -------- | ||
| python merge-results.py shard-0/pr-results.json shard-1/pr-results.json > pr-results.json | ||
| """ | ||
|
|
||
| import json | ||
| import sys | ||
|
|
||
|
|
||
| def merge_results(paths): | ||
| """Sum the per-module summaries in ``paths`` into a single summary.""" | ||
| merged: dict[str, dict] = {} | ||
| for path in paths: | ||
| with open(path) as f: | ||
| results = json.load(f) | ||
| for module_name, row in results.items(): | ||
| combined = merged.setdefault(module_name, {}) | ||
| for key, value in row.items(): | ||
| if isinstance(value, bool): | ||
| # No boolean fields are expected; keep the first seen value. | ||
| combined.setdefault(key, value) | ||
| elif isinstance(value, (int, float)): | ||
| combined[key] = combined.get(key, 0) + value | ||
| else: | ||
| combined.setdefault(key, value) | ||
| return merged | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| print(json.dumps(merge_results(sys.argv[1:]), indent=4)) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| #!/usr/bin/env bash | ||
| # SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| # Shared helper for the sharded pandas-tests jobs. Sourced by summary.sh (PR | ||
| # side) and merge-nightly.sh (nightly side): both download every shard's partial | ||
| # per-module summary from the current run and merge them into one file. The two | ||
| # callers differ only in how they react to a missing shard, so that decision is | ||
| # left to them and this returns non-zero instead of exiting. | ||
|
|
||
| # merge_shard_results <artifact_prefix> <results_filename> <num_shards> <output> | ||
| merge_shard_results() { | ||
| local prefix=$1 filename=$2 num_shards=$3 output=$4 | ||
| local shard | ||
| local results=() | ||
|
|
||
| for ((shard = 0; shard < num_shards; shard++)); do | ||
| if ! gh run download "${GITHUB_RUN_ID}" \ | ||
| --repo "${GITHUB_REPOSITORY}" \ | ||
| --name "${prefix}-${shard}" \ | ||
| --dir "shard-${shard}"; then | ||
| rapids-logger "Could not download results for shard ${shard}." | ||
| return 1 | ||
| fi | ||
| results+=("shard-${shard}/${filename}") | ||
| done | ||
|
|
||
| rapids-logger "Merging ${#results[@]} shard result file(s)" | ||
| python ci/cudf_pandas_scripts/pandas-tests/merge-results.py \ | ||
| "${results[@]}" > "${output}" | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: NVIDIA/cudf
Length of output: 3483
🏁 Script executed:
Repository: NVIDIA/cudf
Length of output: 6425
🌐 Web query:
GitHub Actions inputs context available workflow_dispatch workflow_call push trigger undefined context expression💡 Result:
The inputs context in GitHub Actions is strictly limited to workflows triggered by the workflow_dispatch event (manually) or the workflow_call event (reusable workflows) [1][2][3]. It is not available for the push trigger [4][5]. Key points regarding the availability and behavior of the inputs context: 1. Trigger Availability: The inputs context is only populated when a workflow is triggered by workflow_dispatch or workflow_call [1][2]. If you attempt to reference the inputs context in a workflow triggered by a push event, the values will be undefined (or empty), as the inputs configuration block is not supported for push triggers [4][5]. 2. Handling Multiple Triggers: If a single workflow file defines both workflow_dispatch and other triggers (like push), the inputs context will only contain values when the workflow is manually triggered [3][5]. For non-manual triggers, attempting to access ${{ inputs.<input_name> }} will result in an empty or undefined value [4][5]. 3. Expression Context: Because the inputs context is undefined for push triggers, using expressions like ${{ inputs.my_input }} in those runs will not yield the expected results [5]. Workflow authors typically handle this by using conditional logic based on the event name (e.g., if: ${{ github.event_name == 'push' || inputs.my_input }}) or by separating concerns into different workflows where one acts as a reusable workflow (workflow_call) that is invoked by other workflows [4][5]. 4. Compatibility: While the inputs context is the preferred way to access inputs, values passed via workflow_dispatch are also available in the github.event.inputs context for backward compatibility [6][7][3]. Note that github.event.inputs converts all values to strings, whereas the inputs context preserves types like booleans [6][7].
Citations:
Use a defined input source for
branch,date, andsha.This workflow runs only on
push, so theinputscontext is unavailable. These expressions can resolve to empty values beforecustom-job.yamlreceives them. Use push-event values or declare a supported input trigger. actionlint reports the same issue in the existing jobs.🧰 Tools
🪛 actionlint (1.7.12)
[error] 1000-1000: property "branch" is not defined in object type {}
(expression)
[error] 1001-1001: property "date" is not defined in object type {}
(expression)
[error] 1002-1002: property "sha" is not defined in object type {}
(expression)
🤖 Prompt for AI Agents
Source: Linters/SAST tools
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.
Leaving this one —
inputs.branch/date/shais the existing convention forcustom-job.yamlcallers in this push-triggered workflow (4 jobs onmainalready do it, and actionlint flags those identically). The new job just matches its neighbourpandas-tests; changing only this one would make it inconsistent, so it belongs in a repo-wide cleanup rather than here.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.
@galipremsagar, the consistency rationale is valid. However,inputs.branch,inputs.date, andinputs.sharemain unavailable for this push-triggered workflow.A repository-wide cleanup is appropriate. Please consider opening a follow-up issue to update all affected
custom-job.yamlcallers together. This PR still adds three new invalid references, so this finding remains applicable.You are interacting with an AI system.