-
Notifications
You must be signed in to change notification settings - Fork 916
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
Make improvements in pandas-test reporting #15485
Changes from 11 commits
bfb492b
4e61e1c
67b9877
336f8d3
dace1bd
10f66c0
c383e78
9c5dc90
4299743
aca05cf
5f2c4a0
06e7b6c
b0254f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -130,7 +130,7 @@ jobs: | |
secrets: inherit | ||
uses: rapidsai/shared-workflows/.github/workflows/[email protected] | ||
with: | ||
matrix_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))])) | ||
matrix_filter: map(select(.ARCH == "amd64")) | group_by(.CUDA_VER|split(".")|map(tonumber)|.[0]) | min(max_by([(.PY_VER|split(".")|map(tonumber)), (.CUDA_VER|split(".")|map(tonumber))])) | ||
galipremsagar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
build_type: nightly | ||
branch: ${{ inputs.branch }} | ||
date: ${{ inputs.date }} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,12 +10,13 @@ | |
GH_JOB_NAME="pandas-tests-diff / build" | ||
rapids-logger "Github job name: ${GH_JOB_NAME}" | ||
|
||
MAIN_ARTIFACT=$(rapids-s3-path)cuda12_$(arch)_py310.main-results.json | ||
PR_ARTIFACT=$(rapids-s3-path)cuda12_$(arch)_py39.pr-results.json | ||
PY_VER="39" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same reason as here: https://github.com/rapidsai/cudf/pull/15485/files#r1556345903 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don’t want to hardcode any values here. Our matrix will change over time and break this. Try using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can I open a follow-up for this one? I will need to do some testing here after this PR merges and nightlies are uploaded. |
||
MAIN_ARTIFACT=$(rapids-s3-path)cuda12_$(arch)_py${PY_VER}.main-results.json | ||
PR_ARTIFACT=$(rapids-s3-path)cuda12_$(arch)_py${PY_VER}.pr-results.json | ||
|
||
rapids-logger "Fetching latest available results from nightly" | ||
aws s3api list-objects-v2 --bucket rapids-downloads --prefix "nightly/" --query "sort_by(Contents[?ends_with(Key, '.main-results.json')], &LastModified)[::-1].[Key]" --output text > s3_output.txt | ||
cat s3_output.txt | ||
aws s3api list-objects-v2 --bucket rapids-downloads --prefix "nightly/" --query "sort_by(Contents[?ends_with(Key, '_py${PY_VER}.main-results.json')], &LastModified)[::-1].[Key]" --output text > s3_output.txt | ||
|
||
read -r COMPARE_ENV < s3_output.txt | ||
export COMPARE_ENV | ||
rapids-logger "Latest available results from nightly: ${COMPARE_ENV}" | ||
|
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.
We have the same job in
pr.yaml
and here. One is run on nightly and one on pr's, both have different matrix combinations: https://github.com/rapidsai/shared-workflows/blob/13e008c746e30a830c4ebe83a6786862858dfcb8/.github/workflows/wheels-test.yaml#L75-L94To achieve a consistent matrix for comparison, I changed this to
min
so that we compare withcudf-12.2.2
and python 3.9 versions for both jobs.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.
This is a coincidence that we have the same minimum between the PR matrix and the nightly matrix. Can we find another way to get a common configuration, like using the “latest” image?
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.
Do you think it's okay to hard-code here instead of
map(select..
? Again, can we do this as a follow-up?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.
Sure, feel free to defer on this. Just know that if this workflow breaks when the matrix changes, I’ll come poke you to fix it. 😉