[CI][Refactor] Merge test CI workflow files into one #973
Conversation
|
👋 Hi! Thank you for contributing to the TileLang project. Please remember to run We appreciate you taking this step! Our team will review your contribution, and we look forward to your awesome work! 🚀 |
WalkthroughAdds a matrixed GitHub Actions CI (lint + multi-toolkit tests), refactors several workflows, updates pyproject/build tooling and requirement manifests, makes tests deterministic, adapts example CLIs to accept explicit argv, tweaks a numeric tolerance, and adds a yapf pre-commit hook. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant GH as GitHub Actions
participant Runner as Runner (github-hosted / self-hosted)
participant Setup as Toolkit Setup
participant Build as Build & Install
participant Tests as Test Suites
note right of GH #f0f7ff: CI matrix triggers (lint + toolkit tests)
Dev->>GH: push / open PR / dispatch
GH->>Runner: schedule matrix jobs
Runner->>Setup: Setup Python, caches (ccache/UV), env segregation
Setup->>Build: configure toolkit (CUDA/ROCm/Metal), adjust indices
Build->>Build: build wheel / install project
Build->>Tests: run pytest (toolkit-specific + generic)
Tests-->>GH: upload results & artifacts
GH->>Runner: cleanup
sequenceDiagram
participant Test as Test runner
participant Example as Example main(argv)
Test->>Example: call main(argv=[])
Example->>Example: parser.parse_args(argv)
Example-->>Test: execute with explicit empty args
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
XuehaiPan
force-pushed
the
merge-ci-files
branch
2 times, most recently
from
1245ffd to
be474c9
Compare
XuehaiPan
force-pushed
the
merge-ci-files
branch
2 times, most recently
from
efdf2a9 to
f44e922
Compare
XuehaiPan
force-pushed
the
merge-ci-files
branch
from
f44e922 to
55506a8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
.github/workflows/ci.yml (5)
2-11: Consider running on push to main as wellCurrently only PRs and manual dispatch trigger. If you want tests on merges to main, add push.
on: pull_request: types: - labeled - unlabeled - opened - synchronize - reopened # Allow to trigger the workflow manually workflow_dispatch: + push: + branches: + - main
136-139: Remove unused variableROCM_VERSION_MAJMIN_NODOT is computed but never used.
- ROCM_VERSION_MAJMIN_NODOT="${ROCM_VERSION_MAJMIN//./}"
160-179: Ensure PyTorch extra index is honored by uv pipRelying on env vars may be brittle across uv versions. Pass the index explicitly to guarantee usage, and mirror it for the CUDA/ROCm conditionals.
- uv pip install -r requirements-build.txt -r requirements-test.txt + uv pip install --extra-index-url "${PIP_EXTRA_INDEX_URL:-}" -r requirements-build.txt -r requirements-test.txt if [[ "${{ matrix.runner.toolkit }}" == *"CUDA"* ]]; then - uv pip install flash-attn==2.5.8 + uv pip install --extra-index-url "${PIP_EXTRA_INDEX_URL:-}" flash-attn==2.5.8 elif [[ "${{ matrix.runner.toolkit }}" == *"ROCm"* ]]; then - uv pip install -r requirements-rocm.txt + uv pip install --extra-index-url "${PIP_EXTRA_INDEX_URL:-}" -r requirements-rocm.txtIf uv already respects PIP_EXTRA_INDEX_URL in your runners, feel free to skip; otherwise this removes ambiguity.
54-60: Prefer pre-commit/action for speed and reliability overpipx runpipx may not be present or cached; the official action restores hook caches and is faster.
- - name: Pre-commit Lint - run: | - if ! pipx run pre-commit run --all-files --show-diff-on-failure; then - echo "::error::Pre-commit checks failed. Please run 'pre-commit install' and 'pre-commit run --all-files' locally to see the issues." - exit 1 - fi + - name: Pre-commit Lint + uses: pre-commit/[email protected]If you keep pipx, please confirm pipx is available on ubuntu-latest.
188-206: Optional: run format checks in the lint job to fail fastFormatting is independent of toolchain; moving this to the lint job reduces GPU runner load and speeds feedback.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/amd_ci.yml(0 hunks).github/workflows/ci.yml(1 hunks).github/workflows/metal_ci.yml(0 hunks)
💤 Files with no reviewable changes (2)
- .github/workflows/metal_ci.yml
- .github/workflows/amd_ci.yml
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/ci.yml (2)
189-206: Consider moving format check to the lint job.The format check currently runs on every platform (CUDA, ROCm, Metal), but code formatting is platform-independent. Moving it to the
lintjob would reduce redundancy and save CI time.However, if the format check validates platform-specific CMake configuration, the current approach is appropriate.
If you decide to move it, the format check could run in the lint job after the pre-commit check:
- name: Run format check run: | python -m pip install -r requirements-build.txt mkdir -p build pushd build cmake .. -DCMAKE_EXPORT_COMPILE_COMMANDS=ON popd if ! output=$(./format.sh 2>&1); then echo "::error::Format check failed." echo "$output" exit 1 fi rm -rf build
232-239: Consider adding a timeout to ROCm tests.CUDA and Metal tests specify
--timeout=3600, but ROCm tests (line 239) don't. Adding a timeout ensures consistency and prevents hanging tests from blocking CI indefinitely.Apply this diff to add a timeout:
- python -m pytest -v --cache-clear test_tilelang_test_amd.py + python -m pytest -v --cache-clear --timeout=3600 test_tilelang_test_amd.py
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/amd_ci.yml(0 hunks).github/workflows/ci.yml(1 hunks).github/workflows/metal_ci.yml(0 hunks)
💤 Files with no reviewable changes (2)
- .github/workflows/metal_ci.yml
- .github/workflows/amd_ci.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test for Python 3.12 with ROCm-6.3 (on self-hosted-amd)
- GitHub Check: Test for Python 3.12 with Metal (on macos-latest)
🔇 Additional comments (11)
.github/workflows/ci.yml (11)
2-14: LGTM! Well-structured workflow triggers and permissions.The PR trigger types and workflow_dispatch enable flexible CI execution, and read-only permissions follow security best practices.
34-60: LGTM! Well-structured lint job.The lint job is properly configured with caching, full git history for pre-commit, and a helpful error message to guide developers.
62-82: LGTM! Effective test matrix consolidation.The matrix successfully consolidates the three separate workflows. Using
fail-fast: falseensures all platform failures are visible, and the draft PR check prevents unnecessary resource usage.
101-111: LGTM! Comprehensive venv caching strategy.The cache key properly includes all relevant factors (toolkit version, requirements hash, etc.) to ensure cache validity while maximizing reuse. This aligns with the stated goal of avoiding redundant installations.
113-131: LGTM! Proper CUDA environment configuration.The CUDA setup correctly extracts version information, configures PyTorch indexes, and validates the toolkit installation. The version validation at line 131 appropriately fails the job if nvcc is unavailable.
133-151: LGTM! Consistent ROCm environment configuration.The ROCm setup follows the same pattern as CUDA and correctly configures the ROCm-specific PyTorch index and build options.
153-159: LGTM! Appropriate Metal environment configuration.Metal configuration is correctly simplified compared to CUDA/ROCm since it doesn't require external package indexes.
161-179: LGTM! Well-structured venv setup with proper conditional logic.The venv setup correctly runs only on cache miss and includes appropriate toolkit-specific dependencies. The error handling for unknown toolkits at lines 177-178 is a good safety measure.
208-211: LGTM! Efficient wheel installation.Using
--no-build-isolationis appropriate since build dependencies are already installed in the venv, and the verbose flag aids troubleshooting.
27-27: ConfirmUV_INDEX_STRATEGY="unsafe-best-match"use.
This strategy merges candidates from all configured indexes and selects the highest-versioned release—mirroring pip’s default multi-index behavior but reintroducing dependency-confusion risk. Ensure this behavior is intentional when installing PyTorch wheels from custom indexes.Likely an incorrect or invalid review comment.
213-219: Examples CI runs on CUDA only
The examples folder is entirely CUDA-specific—there are no ROCm or Metal targets in tests or code—so gating this step onmatrix.runner.toolkitcontaining “CUDA” is correct. Add ROCm/Metal jobs when support is available.
|
I think this increase the maintain efforts in some ways: the self-hosted runners seems to be setup in peking university and have a much slower connection with github and pypi/uv. That makes restoring GA cache takes more time than it saves. Considering the different situation for those runners, it might be better to keep them separated? (or use multiple jobs with dependencies, do not merge them in one single job) Some of my previous test could be found around: 76e9df7. I used uv on self-hosted runner locally to speedup package installation, but do not upload them as cache: df9ae25, and uv cache are preserved on host. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
.github/workflows/ci.yml (3)
13-14: Enable actions: write to allow cache saves (setup-python, cache v4)With an explicit permissions block, unspecified scopes (actions) default to none; caches will restore but won’t save. Add actions: write at workflow or job level if you want cache saves.
Based on learnings
Example (workflow-level):
permissions: contents: read + actions: writeIf you intentionally prefer restore-only, ignore. Otherwise, please confirm whether saves are currently occurring in logs.
121-139: Fix CUDA → PyTorch cu tag mapping (avoid non-existent wheel indexes)Deriving cuNNN by stripping dots (e.g., 12.8→cu128) may not exist. Use an explicit map with a sane default.
- CUDA_VERSION="${TOOLKIT##*-}" - CUDA_VERSION_MAJMIN="$(echo ${CUDA_VERSION} | cut -d '.' -f-2)" - CUDA_VERSION_MAJMIN_NODOT="${CUDA_VERSION_MAJMIN//./}" - export PIP_EXTRA_INDEX_URL="https://download.pytorch.org/whl/cu${CUDA_VERSION_MAJMIN_NODOT}" + CUDA_VERSION="${TOOLKIT##*-}" + CUDA_VERSION_MAJMIN="$(echo "${CUDA_VERSION}" | cut -d '.' -f-2)" + # Map CUDA to known PyTorch cu tags (adjust as your supported set evolves) + case "${CUDA_VERSION_MAJMIN}" in + 12.4|12.5|12.6) CU_TAG="cu124" ;; + 12.1|12.2|12.3) CU_TAG="cu121" ;; + 11.8) CU_TAG="cu118" ;; + *) CU_TAG="cu124" ;; # default/fallback + esac + export PIP_EXTRA_INDEX_URL="https://download.pytorch.org/whl/${CU_TAG}" export UV_INDEX="${PIP_EXTRA_INDEX_URL}" export CMAKE_CONFIGURE_OPTIONS="${CMAKE_CONFIGURE_OPTIONS} -DUSE_CUDA=ON" @@ - echo "CUDA_VERSION_MAJMIN_NODOT=${CUDA_VERSION_MAJMIN_NODOT}" | tee -a "${GITHUB_ENV}" + echo "CU_TAG=${CU_TAG}" | tee -a "${GITHUB_ENV}" echo "PIP_EXTRA_INDEX_URL=${PIP_EXTRA_INDEX_URL}" | tee -a "${GITHUB_ENV}" ``` <!-- review_comment_end --> --- `219-226`: **Fix pytest file selection; globstar isn’t enabled by default** The pattern **/test*.py won’t expand. Let pytest discover tests. ```diff - python -m pytest -n 4 **/test*.py -v -r fE --durations=0 --cache-clear + python -m pytest -n 4 -v -r fE --durations=0 --cache-clearAlternatively, run shopt -s globstar before using **.
🧹 Nitpick comments (3)
.github/workflows/ci.yml (3)
141-160: Minor: remove unused ROCm “_NODOT” variable to reduce confusionROCM_VERSION_MAJMIN_NODOT is computed but not used (only echoed). Trim it.
- ROCM_VERSION_MAJMIN_NODOT="${ROCM_VERSION_MAJMIN//./}" @@ - echo "ROCM_VERSION_MAJMIN_NODOT=${ROCM_VERSION_MAJMIN_NODOT}" | tee -a "${GITHUB_ENV}" ``` <!-- review_comment_end --> --- `20-31`: **Self‑hosted runners: prefer local mirrors and host‑persistent caches** Given slower network on self‑hosted runners, consider: - Using a PyPI mirror (and matching UV_INDEX) when on self‑hosted. - Persisting uv/ccache on the host (not GH cache) to avoid slow uploads. - Optionally disabling GH cache for venv/ccache on self‑hosted. Example snippet to set mirrors (add as a step after “Set up Python”): ```yaml - name: Use PyPI mirror on self-hosted (optional) if: contains(join(matrix.runner.tags, ','), 'self-hosted') && env.PYPI_MIRROR_URL != '' env: PYPI_MIRROR_URL: ${{ secrets.PYPI_MIRROR_URL }} run: | echo "PIP_INDEX_URL=${PYPI_MIRROR_URL}" >> "$GITHUB_ENV" echo "UV_INDEX=${PYPI_MIRROR_URL}" >> "$GITHUB_ENV"For ccache, on self‑hosted set CCACHE_DIR to a persistent path (e.g., /var/cache/ccache) and rely on local persistence instead of the GH cache action. Optionally add max-size to cap usage.
189-194: Scope ccache and avoid double‑caching; set a size limit
- Ensure CCACHE_DIR is not under .cache when also using the ccache action to avoid duplicative caching.
- On macOS (Metal), keep ccache; on self‑hosted, consider using a persistent local CCACHE_DIR and skipping the GH cache action.
- Set a max-size to cap cache growth.
Example:
- - name: Setup ccache + - name: Setup ccache uses: hendrikmuhs/ccache-action@v1 with: create-symlink: true key: ccache-${{ runner.os }}-${{ runner.arch }}-${{ matrix.python-version }}-${{ matrix.runner.name }}-${{ matrix.runner.toolkit }} + max-size: 2G ``` <!-- review_comment_end --> <!-- file_end --> </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 2a3e3a167ae0140b99f8ae34a1e086a36684eb44 and 4f796393fd1fa67f9257498a85803f9ee1904607. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `.github/workflows/ci.yml` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (1)</summary> <details> <summary>📚 Learning: 2025-10-10T13:29:29.326Z</summary>Learnt from: XuehaiPan
PR: #973
File: .github/workflows/ci.yml:13-15
Timestamp: 2025-10-10T13:29:29.326Z
Learning: In .github/workflows/ci.yml for tilelang (GitHub Actions), actions/cachev4 and setup-python’s cache feature require GITHUB_TOKEN with actions: write to save caches; with a permissions block that only sets contents: read, unspecified actions permission becomes none, so caches will restore but not save.**Applied to files:** - `.github/workflows/ci.yml` </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)</summary> * GitHub Check: Test for Python 3.12 with Metal (on macos-latest) * GitHub Check: Test for Python 3.12 with ROCm-6.3 (on self-hosted-amd) </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
XuehaiPan
force-pushed
the
merge-ci-files
branch
2 times, most recently
from
7ecda00 to
de88d35
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
.github/workflows/ci.yml (4)
13-14: Enable Actions write so caches can be saved (not just restored)With a permissions block present, unspecified scopes are none. actions/cache@v4 and setup-python caching won’t save without actions: write. Add:
permissions: contents: read + actions: writeBased on learnings. Please confirm in recent runs whether “Saving cache” logs appear for GH-hosted jobs. If not, this change is needed.
144-162: Map CUDA → PyTorch cu tag explicitly; avoid non-existent indicesDeriving cuNNN by stripping dots (e.g., 12.8 → cu128) may not exist. Use a map with fallback.
TOOLKIT="${{ matrix.runner.toolkit }}" CUDA_VERSION="${TOOLKIT##*-}" - CUDA_VERSION_MAJMIN="$(echo ${CUDA_VERSION} | cut -d '.' -f-2)" - CUDA_VERSION_MAJMIN_NODOT="${CUDA_VERSION_MAJMIN//./}" - export PIP_EXTRA_INDEX_URL="https://download.pytorch.org/whl/cu${CUDA_VERSION_MAJMIN_NODOT}" + CUDA_VERSION_MAJMIN="$(echo "${CUDA_VERSION}" | cut -d '.' -f-2)" + case "${CUDA_VERSION_MAJMIN}" in + 12.4|12.5|12.6) CU_TAG="cu124" ;; + 12.1|12.2|12.3) CU_TAG="cu121" ;; + 11.8) CU_TAG="cu118" ;; + *) CU_TAG="cu124" ;; + esac + export PIP_EXTRA_INDEX_URL="https://download.pytorch.org/whl/${CU_TAG}" export UV_INDEX="${PIP_EXTRA_INDEX_URL}" export CMAKE_CONFIGURE_OPTIONS="${CMAKE_CONFIGURE_OPTIONS} -DUSE_CUDA=ON" @@ - echo "CUDA_VERSION_MAJMIN_NODOT=${CUDA_VERSION_MAJMIN_NODOT}" | tee -a "${GITHUB_ENV}" + echo "CU_TAG=${CU_TAG}" | tee -a "${GITHUB_ENV}" echo "PIP_EXTRA_INDEX_URL=${PIP_EXTRA_INDEX_URL}" | tee -a "${GITHUB_ENV}"
200-206: Avoid stale venvs: move CUDA-only deps into a tracked requirements fileInline installing flash-attn bypasses your venv cache key (which hashes only requirements*.txt). Commit a requirements-cuda.txt and install from it.
- if [[ "${{ matrix.runner.toolkit }}" == "CUDA"* ]]; then - uv pip install flash-attn==2.5.8 + if [[ "${{ matrix.runner.toolkit }}" == "CUDA"* ]]; then + uv pip install -r requirements-cuda.txt
258-265: Fix pytest file selection; remove**/test*.py(globstar not enabled)Bash on runners doesn’t enable globstar; pattern won’t expand. Let pytest discover tests.
- python -m pytest -n 4 **/test*.py -v -r fE --durations=0 --cache-clear + python -m pytest -n 4 -v -r fE --durations=0 --cache-clear
🧹 Nitpick comments (2)
.github/workflows/ci.yml (2)
136-143: Skip actions/cache on self-hosted to save timeThis step runs even on self-hosted (where you use your own tar cache). Guard it:
- - name: Setup venv cache (GitHub-hosted runners) + - name: Setup venv cache (GitHub-hosted runners) + if: ! startsWith(matrix.runner.name, 'self-hosted')
106-114: Mirror indexes for self-hosted runners (slow network)For self-hosted, set PIP_INDEX_URL/UV_INDEX from a mirror or secret to mitigate slow external bandwidth:
- name: Setup venv cache (self-hosted runners) if: startsWith(matrix.runner.name, 'self-hosted') id: venv-cache-self-hosted run: | + # Optional: speed up installs on self-hosted with a local/mirror index + if [[ -n "${PIP_MIRROR_URL:-}" ]]; then + echo "Using pip mirror: ${PIP_MIRROR_URL}" + export PIP_INDEX_URL="${PIP_MIRROR_URL}" + export UV_INDEX="${PIP_MIRROR_URL}" + fiSet PIP_MIRROR_URL via repo/org secrets or environments per runner.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-10T13:29:29.326Z
Learnt from: XuehaiPan
PR: tile-ai/tilelang#973
File: .github/workflows/ci.yml:13-15
Timestamp: 2025-10-10T13:29:29.326Z
Learning: In .github/workflows/ci.yml for tilelang (GitHub Actions), actions/cachev4 and setup-python’s cache feature require GITHUB_TOKEN with actions: write to save caches; with a permissions block that only sets contents: read, unspecified actions permission becomes none, so caches will restore but not save.
Applied to files:
.github/workflows/ci.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test for Python 3.12 with ROCm-6.3 (on self-hosted-amd)
- GitHub Check: Test for Python 3.12 with Metal (on macos-latest)
Hi @oraluben, if I understand correctly, you mean downloading/uploading the I have implemented a manual caching mechanism in commit de88d35 with some code comments. I believe it will resolve your concerns. The new |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/ci.yml (2)
13-14: Reinstateactions: writeso caches can saveWith an explicit permissions block that omits
actions, GitHub zeros that scope; actions/cache@v4 and setup-python’s pip cache can restore but cannot save (403 on upload). We need writable Actions perms for caching to pay off. Please addactions: writeback.
148-160: Don’t fabricatecu${CUDA_VERSION}tags; map to real PyTorch wheelsStripping the dot turns 12.8 into
cu128, which PyTorch doesn’t publish. Installs will miss the correct wheel and fall back (or fail). Please map supported CUDA versions to actual cu tags with acase/lookup and fall back sanely. Example:- CUDA_VERSION_MAJMIN_NODOT="${CUDA_VERSION_MAJMIN//./}" - export PIP_EXTRA_INDEX_URL="https://download.pytorch.org/whl/cu${CUDA_VERSION_MAJMIN_NODOT}" + case "${CUDA_VERSION_MAJMIN}" in + 12.4|12.5|12.6|12.7|12.8) CU_TAG="cu124" ;; + 12.1|12.2|12.3) CU_TAG="cu121" ;; + 11.8) CU_TAG="cu118" ;; + *) CU_TAG="cpu" ;; + esac + export PIP_EXTRA_INDEX_URL="https://download.pytorch.org/whl/${CU_TAG}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-10T13:29:29.326Z
Learnt from: XuehaiPan
PR: tile-ai/tilelang#973
File: .github/workflows/ci.yml:13-15
Timestamp: 2025-10-10T13:29:29.326Z
Learning: In .github/workflows/ci.yml for tilelang (GitHub Actions), actions/cachev4 and setup-python’s cache feature require GITHUB_TOKEN with actions: write to save caches; with a permissions block that only sets contents: read, unspecified actions permission becomes none, so caches will restore but not save.
Applied to files:
.github/workflows/ci.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test for Python 3.12 with Metal (on macos-latest)
- GitHub Check: Test for Python 3.12 with ROCm-6.3 (on self-hosted-amd)
🔇 Additional comments (1)
.github/workflows/ci.yml (1)
202-211: Fix toolkit detection: quoted glob makes every branch missIn
[[ … == "CUDA"* ]]the quotes make*literal, so"CUDA-12.8"never matches — we fall through to the finalelseand abort on every CUDA run (same for ROCm/Metal). Drop the quotes or switch tocase/regex. For example:- if [[ "${{ matrix.runner.toolkit }}" == "CUDA"* ]]; then + if [[ "${{ matrix.runner.toolkit }}" == CUDA* ]]; then … - elif [[ "${{ matrix.runner.toolkit }}" == "ROCm"* ]]; then + elif [[ "${{ matrix.runner.toolkit }}" == ROCm* ]]; then … - elif [[ "${{ matrix.runner.toolkit }}" == "Metal"* ]]; then + elif [[ "${{ matrix.runner.toolkit }}" == Metal* ]]; thenLikely an incorrect or invalid review comment.
XuehaiPan
force-pushed
the
merge-ci-files
branch
2 times, most recently
from
1226fa2 to
0badbdd
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
.github/workflows/ci.yml (2)
204-211: Pattern match still treats self-hosted as GitHub-hostedQuoting
"self-hosted"*disables wildcard matching, so the condition stays true and self-hosted runners wrongly drop into the ccache branch (exact issue previously reported). Please remove the quotes or switch to acasesoself-hosted-*hits the else path as intended.- if [[ "${{ matrix.runner.name }}" != "self-hosted"* ]]; then + if [[ ${matrix.runner.name} != self-hosted* ]]; then
13-14: Addactions: writeso caches actually persistSpecifying only
permissions: contents: readzeros theactionsscope, so neitheractions/cache@v4nor thesetup-pythonpip cache can save artifacts—every run will reinstall from scratch. Please addactions: write(workflow or job level) alongsidecontents: readto restore cache saves. Based on learningspermissions: contents: read + actions: write
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-10T13:29:29.326Z
Learnt from: XuehaiPan
PR: tile-ai/tilelang#973
File: .github/workflows/ci.yml:13-15
Timestamp: 2025-10-10T13:29:29.326Z
Learning: In .github/workflows/ci.yml for tilelang (GitHub Actions), actions/cachev4 and setup-python’s cache feature require GITHUB_TOKEN with actions: write to save caches; with a permissions block that only sets contents: read, unspecified actions permission becomes none, so caches will restore but not save.
Applied to files:
.github/workflows/ci.yml
XuehaiPan
force-pushed
the
merge-ci-files
branch
2 times, most recently
from
2c10759 to
0c054e3
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/ci.yml (1)
13-14: Allow cache saves by grantingactions: write.With
permissions:explicitly set to onlycontents: read, every other scope drops tonone, so actions/cache@v4 (and setup-python’s pip cache) can restore but cannot save. That knocks out the speedup this PR aims for on GitHub-hosted runners. Please addactions: write(workflow- or job-level) so caches persist between runs. Based on learnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-10T13:29:29.326Z
Learnt from: XuehaiPan
PR: tile-ai/tilelang#973
File: .github/workflows/ci.yml:13-15
Timestamp: 2025-10-10T13:29:29.326Z
Learning: In .github/workflows/ci.yml for tilelang (GitHub Actions), actions/cachev4 and setup-python’s cache feature require GITHUB_TOKEN with actions: write to save caches; with a permissions block that only sets contents: read, unspecified actions permission becomes none, so caches will restore but not save.
Applied to files:
.github/workflows/ci.yml
XuehaiPan
force-pushed
the
merge-ci-files
branch
3 times, most recently
from
c9160bd to
98ffe4e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
.github/workflows/ci.yml (3)
13-14: Restore cache saves by grantingactions: write.With an explicit
permissions:block, anything you omit (includingactions) is forced tonone. That meansactions/cache@v4(used by setup-uv/setup-python) can restore but can’t save caches, defeating the caching strategy for GitHub-hosted runners. Addactions: writehere (or at job level) so cache uploads resume. Based on learnings
129-145: Map CUDA versions to real PyTorch CU tags.Deriving
cu${CUDA_VERSION_MAJMIN_NODOT}turns CUDA 12.8 intocu128, which PyTorch doesn’t publish, so wheel installs fail. Use an explicit version→cu tag map (with a safe fallback) instead of string stripping to ensure a valid index URL for each supported toolkit.
251-278: Correctuv runinvocations (-mneeds the module name).
uv run --no-project -m -- pytest …passes an empty module (--) to-m, so the command exits before pytest starts. Move the module name immediately after-m(e.g.,uv run --no-project -m pytest -- …) or calluv run --no-project python -m pytest …in every pytest step here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-10T13:29:29.347Z
Learnt from: XuehaiPan
PR: tile-ai/tilelang#973
File: .github/workflows/ci.yml:13-15
Timestamp: 2025-10-10T13:29:29.347Z
Learning: In .github/workflows/ci.yml for tilelang (GitHub Actions), actions/cachev4 and setup-python’s cache feature require GITHUB_TOKEN with actions: write to save caches; with a permissions block that only sets contents: read, unspecified actions permission becomes none, so caches will restore but not save.
Applied to files:
.github/workflows/ci.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test for Python 3.12 with ROCm-6.3 (on self-hosted-amd)
- GitHub Check: Test for Python 3.12 with CUDA-12.8 (on self-hosted-nvidia)
- GitHub Check: Test for Python 3.12 with Metal (on macos-latest)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
requirements.txt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test for Python 3.12 with Metal (on macos-latest)
- GitHub Check: Test for Python 3.12 with Nightly-ROCm-7.0 (on self-hosted-amd)
- GitHub Check: Test for Python 3.12 with CUDA-12.8 (on self-hosted-nvidia)
- GitHub Check: Build wheels for Python 3.8 on ubuntu-latest with CUDA-12.1
- GitHub Check: Build wheels for Python 3.8 on macos-latest with Metal
- GitHub Check: Build wheels for Python 3.8 on ubuntu-24.04-arm with CUDA-12.8
XuehaiPan
force-pushed
the
merge-ci-files
branch
from
0bbfa1e to
a38baf9
Compare
|
cc @Alex4210987 for the rocm ci issues. |
XuehaiPan
force-pushed
the
merge-ci-files
branch
from
e3ce4c3 to
7b7280f
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
.github/workflows/ci.yml (2)
274-374: Fixuv run -m -- pytest(module missing, tests never run)Every invocation
uv run --no-project -m -- …passes an empty module, souvexits before running pytest/collect_env. Put the module right after-m(e.g.,uv run --no-project python -m pytest …anduv run --no-project python -m torch.utils.collect_env). Update all blocks here so tests actually execute.
13-15: Grantactions: writeso caches can saveWith only
contents: readspecified, the GITHUB_TOKEN’sactionsscope defaults tonone, sosetup-uv/actions-cache@v4restores caches but can’t save them. Addactions: write(orwrite-all) to persist Python/venv caches. (Based on learnings)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci.yml(1 hunks).github/workflows/rocm-ci.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-10T13:29:29.347Z
Learnt from: XuehaiPan
PR: tile-ai/tilelang#973
File: .github/workflows/ci.yml:13-15
Timestamp: 2025-10-10T13:29:29.347Z
Learning: In .github/workflows/ci.yml for tilelang (GitHub Actions), actions/cachev4 and setup-python’s cache feature require GITHUB_TOKEN with actions: write to save caches; with a permissions block that only sets contents: read, unspecified actions permission becomes none, so caches will restore but not save.
Applied to files:
.github/workflows/ci.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test for Python 3.12 with Metal (on macos-latest)
- GitHub Check: Test for Python 3.12 with Nightly-ROCm-7.0 (on self-hosted-amd)
- GitHub Check: Build wheels for Python 3.8 on macos-latest with Metal
- GitHub Check: Build wheels for Python 3.8 on ubuntu-24.04-arm with CUDA-12.8
- GitHub Check: Build wheels for Python 3.8 on ubuntu-latest with CUDA-12.1
XuehaiPan
force-pushed
the
merge-ci-files
branch
from
7b7280f to
1758297
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
.github/workflows/ci.yml (2)
13-15: Allow caches to save (actions: writerequired).With an explicit
permissionsblock, every omitted scope becomesnone. That leaves the Actions permission unset, soactions/cache@v4,setup-python’s pip cache, andsetup-uv’s cache can restore but never save. Re-enable the write scope so GitHub-hosted runs keep their caches.permissions: contents: read + actions: writeBased on learnings
145-167: Fix CUDA→PyTorch index (cu128 doesn’t exist).
CUDA-12.8currently yieldscu128, but PyTorch only ships cu118/cu121/cu124… This 404s and leaves CUDA wheels unset, breaking the CUDA job. Map CUDA MAJ.MIN to a real cu tag (with an error for unknown versions) before exporting the index.CUDA_VERSION="${TOOLKIT##*-}" CUDA_VERSION_MAJMIN="$(echo ${CUDA_VERSION} | cut -d '.' -f-2)" - CUDA_VERSION_MAJMIN_NODOT="${CUDA_VERSION_MAJMIN//./}" - if [[ "${TOOLKIT}" == "Nightly-"* ]]; then - # Use torch nightly builds - export PIP_EXTRA_INDEX_URL="https://download.pytorch.org/whl/nightly/cu${CUDA_VERSION_MAJMIN_NODOT}" - else - export PIP_EXTRA_INDEX_URL="https://download.pytorch.org/whl/cu${CUDA_VERSION_MAJMIN_NODOT}" - fi + case "${CUDA_VERSION_MAJMIN}" in + 12.4|12.5|12.6|12.7|12.8) CU_TAG="cu124" ;; + 12.1|12.2|12.3) CU_TAG="cu121" ;; + 11.8) CU_TAG="cu118" ;; + *) + echo "::error::Unsupported CUDA version ${CUDA_VERSION}. Update the cu tag map." + exit 1 + ;; + esac + if [[ "${TOOLKIT}" == Nightly-* ]]; then + export PIP_EXTRA_INDEX_URL="https://download.pytorch.org/whl/nightly/${CU_TAG}" + else + export PIP_EXTRA_INDEX_URL="https://download.pytorch.org/whl/${CU_TAG}" + fi export UV_INDEX="${PIP_EXTRA_INDEX_URL}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci.yml(1 hunks).github/workflows/rocm-ci.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/rocm-ci.yml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-10T13:29:29.347Z
Learnt from: XuehaiPan
PR: tile-ai/tilelang#973
File: .github/workflows/ci.yml:13-15
Timestamp: 2025-10-10T13:29:29.347Z
Learning: In .github/workflows/ci.yml for tilelang (GitHub Actions), actions/cachev4 and setup-python’s cache feature require GITHUB_TOKEN with actions: write to save caches; with a permissions block that only sets contents: read, unspecified actions permission becomes none, so caches will restore but not save.
Applied to files:
.github/workflows/ci.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test for Python 3.12 with Metal (on macos-latest)
- GitHub Check: Build wheels for Python 3.8 on ubuntu-24.04-arm with CUDA-12.8
- GitHub Check: Build wheels for Python 3.8 on ubuntu-latest with CUDA-12.1
- GitHub Check: Build wheels for Python 3.8 on macos-latest with Metal
- GitHub Check: build-test-amd
XuehaiPan
force-pushed
the
merge-ci-files
branch
2 times, most recently
from
5bd3a21 to
6b22d3a
Compare
XuehaiPan
force-pushed
the
merge-ci-files
branch
from
6b22d3a to
ab98648
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
.github/workflows/rocm-ci.yml (1)
113-115: Still need to guardtorch.utils.collect_env.This reintroduces the earlier failure path: on a cold venv (or when the requirements set doesn’t install torch),
python -m torch.utils.collect_envraisesModuleNotFoundError, and withset -ethe whole job exits early. Please gate the call on torch being importable so the job survives cache misses.- echo "::group::torch.utils.collect_env" - python -m torch.utils.collect_env - echo "::endgroup::" + if python - <<'PYCODE' +import importlib.util, sys +sys.exit(0 if importlib.util.find_spec("torch") else 1) +PYCODE + then + echo "::group::torch.utils.collect_env" + python -m torch.utils.collect_env + echo "::endgroup::" + else + echo "torch not installed yet; skip collect_env for this run" + fi.github/workflows/ci.yml (1)
13-15: Enable Actions write permission for cachesWith
permissionsexplicitly set, every unspecified scope (includingactions) becomesnone.actions/cache@v4,setup-python’s pip cache, andsetup-uv’s cache uploads therefore fail to save, so every run misses cache hits. Grantactions: write(workflow- or job-level) to restore upload capability.permissions: contents: read + actions: write
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci.yml(1 hunks).github/workflows/rocm-ci.yml(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-10T13:29:29.347Z
Learnt from: XuehaiPan
PR: tile-ai/tilelang#973
File: .github/workflows/ci.yml:13-15
Timestamp: 2025-10-10T13:29:29.347Z
Learning: In .github/workflows/ci.yml for tilelang (GitHub Actions), actions/cachev4 and setup-python’s cache feature require GITHUB_TOKEN with actions: write to save caches; with a permissions block that only sets contents: read, unspecified actions permission becomes none, so caches will restore but not save.
Applied to files:
.github/workflows/ci.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test for Python 3.12 with Metal (on macos-latest)
- GitHub Check: Test for Python 3.12 with Nightly-ROCm-7.0 (on self-hosted-amd)
- GitHub Check: Test for Python 3.12 with CUDA-12.8 (on self-hosted-nvidia)
- GitHub Check: Build wheels for Python 3.8 on ubuntu-latest with CUDA-12.1
- GitHub Check: Build wheels for Python 3.8 on macos-latest with Metal
- GitHub Check: Build wheels for Python 3.8 on ubuntu-24.04-arm with CUDA-12.8
|
@XuehaiPan hi,you should set different env var for each version. |
| echo "USE_ROCM=ON" | tee -a "${GITHUB_ENV}" | ||
| echo "ROCM_VERSION=${ROCM_VERSION}" | tee -a "${GITHUB_ENV}" | ||
| echo "ROCM_VERSION_MAJMIN=${ROCM_VERSION_MAJMIN}" | tee -a "${GITHUB_ENV}" | ||
| echo "ROCM_VERSION_MAJMIN_NODOT=${ROCM_VERSION_MAJMIN_NODOT}" | tee -a "${GITHUB_ENV}" | ||
| echo "PIP_EXTRA_INDEX_URL=${PIP_EXTRA_INDEX_URL}" | tee -a "${GITHUB_ENV}" | ||
| echo "UV_INDEX=${UV_INDEX}" | tee -a "${GITHUB_ENV}" |
There was a problem hiding this comment.
For example,USE_ROCM=True, for rocm ci.
@Alex4210987 It is defined here.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/ci.yml (1)
13-15: Grantactions: writeso caches can saveWith an explicit
permissions:block, omitted scopes drop tonone.actions/cache@v4andsetup-python’s cache needactions: writeto upload; today the workflow restores but never saves, defeating the cache. Add the missing scope at workflow or job level. Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-10T13:29:29.347Z
Learnt from: XuehaiPan
PR: tile-ai/tilelang#973
File: .github/workflows/ci.yml:13-15
Timestamp: 2025-10-10T13:29:29.347Z
Learning: In .github/workflows/ci.yml for tilelang (GitHub Actions), actions/cachev4 and setup-python’s cache feature require GITHUB_TOKEN with actions: write to save caches; with a permissions block that only sets contents: read, unspecified actions permission becomes none, so caches will restore but not save.
Applied to files:
.github/workflows/ci.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test for Python 3.12 with Metal (on macos-latest)
- GitHub Check: Test for Python 3.12 with CUDA-12.8 (on self-hosted-nvidia)
- GitHub Check: Test for Python 3.12 with Nightly-ROCm-7.0 (on self-hosted-amd)
- GitHub Check: Build wheels for Python 3.8 on macos-latest with Metal
- GitHub Check: Build wheels for Python 3.8 on ubuntu-24.04-arm with CUDA-12.8
- GitHub Check: Build wheels for Python 3.8 on ubuntu-latest with CUDA-12.1
|
LGTM, thanks for your great job @XuehaiPan ! and thanks for @oraluben 's review. |
5 participants
This PR is a follow-up PR to:
Please review and merge that first if you are interested in this one.
This PR merges the three workflow files for supported platforms (currently CUDA, ROCm, and Metal) into a single file with matrix strategies.
This enables several advantages:
Summary by CodeRabbit
Chores
Refactor
Tests
Style
Config