-
Notifications
You must be signed in to change notification settings - Fork 183
[build](feat) Added build entry setup_ascend.py #1485
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: main
Are you sure you want to change the base?
Changes from all commits
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -78,7 +78,7 @@ jobs: | |||||||||||||||||||||||||||||||||||||||||||||||||||||
| NUM_PROCS=$(( $(nproc) / 3 )) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if [ "$NUM_PROCS" -lt 1 ]; then NUM_PROCS=1; fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cd base | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MAX_JOBS="$NUM_PROCS" python setup.py bdist_wheel --dist-dir="${GITHUB_WORKSPACE}/base/wheelhouse" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MAX_JOBS="$NUM_PROCS" python setup_ascend.py bdist_wheel --dist-dir="${GITHUB_WORKSPACE}/base/wheelhouse" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
80
to
+81
Contributor
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. [bug · critical] The "Checkout base" step (line 53) checks out the base/target branch at Meanwhile, the base branch's Fix: Revert this line back to
Comment on lines
80
to
+81
Contributor
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.
The "Build triton-ascend base" step (line 81) runs in the In contrast, the parallel change in Suggested fix: Suggestion:
Suggested change
Comment on lines
80
to
+81
Contributor
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.
The "Build triton-ascend base" step (line 81) runs in the In contrast, the parallel change in Suggested fix: Suggestion:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Build triton-ascend pr | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| shell: bash | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -98,7 +98,7 @@ jobs: | |||||||||||||||||||||||||||||||||||||||||||||||||||||
| NUM_PROCS=$(( $(nproc) / 3 )) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if [ "$NUM_PROCS" -lt 1 ]; then NUM_PROCS=1; fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cd pr | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MAX_JOBS="$NUM_PROCS" python setup.py bdist_wheel --dist-dir="${GITHUB_WORKSPACE}/pr/wheelhouse" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MAX_JOBS="$NUM_PROCS" python setup_ascend.py bdist_wheel --dist-dir="${GITHUB_WORKSPACE}/pr/wheelhouse" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
100
to
+101
Contributor
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. [bug · low] However, for consistency and to avoid future breakage (if the file name changes again), consider adopting the same defensive
Comment on lines
100
to
+101
Contributor
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. [bug · low] The PR checkout contains the newly added However, for defensive programming and consistency with |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: generate ttadapter | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| shell: bash | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| from setup_ascend import _get_triton_ascend_patch_file, _checkout_file | ||
|
|
||
| patch_files, dev_patch_files = _get_triton_ascend_patch_file() | ||
| if dev_patch_files: | ||
| _checkout_file(dev_patch_files) | ||
| if patch_files: | ||
| _checkout_file(patch_files) |
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.
[bug · critical]
Critical bug:
setup_ascend.pydoes not exist in the base branch checkout.The "Checkout base" step (line 53) checks out the base/target branch via
ref: ${{ github.base_ref }}. Sincesetup_ascend.pyis a new file added by this PR (listed as ADDED in the changed files), it does not exist in the base branch. The build stepcd base && python setup_ascend.py bdist_wheel ...will fail with aFileNotFoundError.Comparison with other workflow fixes:
The other modified workflow
Ascend950-ci.ymlwisely uses a conditional check:Suggestion: Apply the same pattern here. For the base branch build, fall back to
setup.pysincesetup_ascend.pywon't be there. Alternatively, only usesetup_ascend.pyfor the PR checkout (which contains the new file) and keep usingsetup.pyfor the base checkout.