-
Notifications
You must be signed in to change notification settings - Fork 22
[CI] Automate AITER prebuilt upload flow with GitHub actions #412
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
base: dev
Are you sure you want to change the base?
Conversation
|
This approach does not work.
|
|
I like your idea of creating a separate GHA workflow for this. I feel creating this workflow which accepts a docker image is a better option. I don't see lot of our group is pushing to the aiter artifactory. Creating this workflow will be a good option to increase the cache hit. As per the permissions, we can explore this option |
Add a workflow_dispatch GHA to build/upload aiter prebuilts using a chosen image and GPU arch list, reusing the shell script. Make ci/aiter_upload.sh handle build + package + upload with optional env-based upload, respecting GPU_ARCHS input and defaulting to gfx942;gfx950. Strip upload/packaging logic out of the CMake helper so normal builds only download/use prebuilts.
69e8d2e to
a631973
Compare
|
@ipanfilo @wangye805 PR is ready for review. Summary: Offloaded all upload related functionality from cmake file. Added a new bash script to upload and this is used in the github action file. |
ci/aiter_upload.sh
Outdated
| ROCM_VER="$(head -n1 "${ROCM_PATH}/.info/version" | sed -n 's/^\([0-9]\+\.[0-9]\+\).*/\1/p')" | ||
|
|
||
| AITER_DIR="${ROOT_DIR}/3rdparty/aiter" | ||
| git -C "${AITER_DIR}" config --global --add safe.directory "${AITER_DIR}" >/dev/null |
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.
Use code from
| COMMAND sh -c "export GIT_CONFIG_GLOBAL=$(mktemp /tmp/gitconfig.XXXXXX); |
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.
Updated
| echo "[AITER-PREBUILT] Building aiter libs for ${ARCHS} ..." | ||
| rm -rf "${AITER_DIR}/aiter/jit/build" | ||
| AITER_LOG_MORE=1 \ | ||
| GPU_ARCHS="${ARCHS}" \ |
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.
CK_TILE_FLOAT_TO_BFLOAT16_DEFAULT is missed
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.
Updated
| -T "${OUTPUT_TGZ}" \ | ||
| "${REMOTE_URL}" \ | ||
| -o /dev/null | ||
| echo "[AITER-PREBUILT] Uploaded tgz to ${REMOTE_URL}" |
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.
What about .sha256?
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.
Jfrog is automatically creating a sha256 if we upload a file, so to leverage thatI didn't upload. But in future if we change the storage loc, we might need, so I will upload it.
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.
If JFrog creates sha256 then it may make more sense to download it and compare with local sha256 to make sure the package uploaded correctly. And if there is no sha256 on server and you want support non-Jfrog servers then upload local sha256 file
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.
Updated, added functionality to check the sha256 with local
| @@ -0,0 +1,92 @@ | |||
| name: AITER Prebuilt Upload | |||
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.
Add copyright
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.
Updated
|
|
||
| - name: Host Diagnostics (upload) | ||
| run: | | ||
| echo "::group::Host Diagnostics" |
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.
For building this host info is not important
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.
Updated
| docker exec \ | ||
| -e NVTE_AITER_PREBUILT_BASE_URL=${NVTE_AITER_PREBUILT_BASE_URL} \ | ||
| -e NVTE_AITER_PREBUILT_UPLOAD_TOKEN=${NVTE_AITER_PREBUILT_UPLOAD_TOKEN} \ | ||
| -e GPU_ARCHS_INPUT="${{ inputs.gpu_archs }}" \ |
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.
Why this intermediate var is needed?
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.
Updated
| exit 1 | ||
| fi | ||
| docker exec \ |
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.
No need to separate run and exec
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.
Updated
| ci/aiter_upload.sh --build | ||
| ' | ||
| - name: Cleanup container |
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.
If using run --rm, this step is not needed
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.
Updated
| @@ -0,0 +1,81 @@ | |||
| #!/usr/bin/env bash | |||
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.
Add copyright. And maybe move it to .github/scripts/
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.
Updated, moved to .github/scripts
Move aiter upload helper to .github/scripts, add copyright header, and use a temp gitconfig for safe.directory/commit lookup set CK_TILE_FLOAT_TO_BFLOAT16_DEFAULT, added functionality to verify remote SHA after upload Trim workflow diagnostics/cleanup, use --rm container, pass GPU_ARCHS input directly
Description
Fixes # (issue)
Type of change
Changes
Checklist: