-
Notifications
You must be signed in to change notification settings - Fork 217
CI: Fix pathfinder CI infrastructure and remove redundant component testing #904
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
CI: Fix pathfinder CI infrastructure and remove redundant component testing #904
Conversation
Co-authored-by: leofang <5534781+leofang@users.noreply.github.com>
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.
@copilot address the comments above |
Co-authored-by: leofang <5534781+leofang@users.noreply.github.com>
/ok to test c110dcf |
@copilot please remove all pathfinder test steps from the test workflows. As mentioned we only develop (and thus) pathfinder on the main branch. So for the backport branch, the focus is to ensure cuda.bindings 12.x can run correctly with the latest pathfinder. |
Co-authored-by: leofang <5534781+leofang@users.noreply.github.com>
Removed all pathfinder test steps from both Linux and Windows test workflows as requested. The workflows now focus on ensuring cuda.bindings 12.x works correctly with the latest pathfinder from main, without redundantly testing pathfinder itself. Fixed in commit 7f46e6c. |
/ok to test 7f46e6c |
@copilot for the same reasons, we should remove the cuda.core tests from both test workflows. Only cuda-bindings and cuda-python 12.x are maintained in this branch. |
Co-authored-by: leofang <5534781+leofang@users.noreply.github.com>
Removed all cuda.core test steps from both Linux and Windows test workflows as requested. The workflows now focus only on cuda-bindings and cuda-python 12.x testing, which are the components maintained in this branch. Fixed in commit 402759e. |
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.
Just a question about a potential simplification. If it looks interesting, we could do that in a separate PR. I see the code is just moved around in this PR.
- name: Install GitHub CLI | ||
# gh is needed for artifact fetching in subsequent steps | ||
run: | | ||
# See https://github.com/cli/cli/blob/trunk/docs/install_linux.md#debian-ubuntu-linux-raspberry-pi-os-apt. | ||
mkdir -p -m 755 /etc/apt/keyrings \ | ||
&& out=$(mktemp) && wget -nv -O$out https://cli.github.com/packages/githubcli-archive-keyring.gpg \ | ||
&& cat $out | tee /etc/apt/keyrings/githubcli-archive-keyring.gpg > /dev/null \ | ||
&& chmod go+r /etc/apt/keyrings/githubcli-archive-keyring.gpg \ | ||
&& echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/githubcli-archive-keyring.gpg] https://cli.github.com/packages stable main" | tee /etc/apt/sources.list.d/github-cli.list > /dev/null \ | ||
&& apt update \ | ||
&& apt install gh -y |
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.
Did you look into using an existing action?
This is from ChatGPT, I didn't try it out:
- name: Setup GitHub CLI
uses: actions4gh/setup-gh@v1
with:
gh-version: latest # or specify "2.65.0", etc.
token: ${{ secrets.GITHUB_TOKEN }}
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.
It seems setup-gh is no longer maintained... 😢 https://github.com/actions4gh/setup-gh
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.
FWIW we already have a install_unix_deps
action. Perhaps we could just move this bits there to allow using it to install gh
?
/ok to test 402759e |
Fixes #810 by updating both Linux and Windows test workflows to always fetch the
cuda-pathfinder
wheel from the latest successful run on the main branch, rather than from the current branch's build artifacts. Additionally removes redundant testing of components that are not actively maintained in this branch.Problem
The current CI design has issues with outdated artifacts and redundant testing:
Outdated pathfinder artifacts: The 12.9.x branch test workflows download
cuda-pathfinder
from the same branch as other components. However,cuda-pathfinder
is only actively maintained on the main branch, so using artifacts from the 12.9.x branch could lead to outdated or inconsistent pathfinder functionality.Redundant component testing: The workflows run tests for
cuda-pathfinder
andcuda.core
on backport branches, but since these components are only developed on main, these tests are redundant and don't add value.Solution
This PR modifies both
test-wheel-linux.yml
andtest-wheel-windows.yml
to:cuda-pathfinder
with new steps that fetch from the main branchThe new pathfinder download steps:
cuda-pathfinder-wheel
artifact from that run./cuda_pathfinder
directoryKey Features
This change ensures that 12.9.x branch testing always uses the maintained implementations from main for compatibility testing with cuda.bindings, while eliminating redundant testing and the risk of using stale code from components not maintained in this branch.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.