ci(gpu): native self-hosted GPU build + test workflow - #1044
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a "gpu-only" test preset and updates the CMake configuration to ensure host-compiled consumers of Cytnx public headers (like gpu_test_main) can correctly locate CUDA and CCCL headers. It also tags discovered GPU tests with the gpu label and sets the discovery mode to PRE_TEST. The review feedback recommends wrapping the CUDAToolkit_INCLUDE_DIRS include directory command in an if condition to prevent a CMake syntax error if the variable is empty.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| # and CCCL include dirs PRIVATEly, so they do not propagate here through | ||
| # linking; add them explicitly, otherwise the host compile fails with | ||
| # "fatal error: cuda/std/complex: No such file or directory". | ||
| target_include_directories(gpu_test_main PRIVATE ${CUDAToolkit_INCLUDE_DIRS}) |
There was a problem hiding this comment.
If CUDAToolkit_INCLUDE_DIRS is empty or undefined (which can happen in some environments where CUDA is installed in standard system paths), the command expands to target_include_directories(gpu_test_main PRIVATE), which is a CMake syntax error and will cause the configuration step to fail. Wrapping it in an if check ensures that the directory is only added if the variable is non-empty, preventing potential configuration failures.
if(CUDAToolkit_INCLUDE_DIRS)
target_include_directories(gpu_test_main PRIVATE ${CUDAToolkit_INCLUDE_DIRS})
endif()
There was a problem hiding this comment.
Addressed in e9e692c — wrapped the CUDAToolkit_INCLUDE_DIRS include in an if() guard (mirroring the CYTNX_CCCL_INCLUDE_DIR guard right below it), so an empty/undefined variable no longer expands to an empty target_include_directories() call and fail configuration.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1044 +/- ##
=======================================
Coverage 73.06% 73.06%
=======================================
Files 224 224
Lines 27642 27642
Branches 71 71
=======================================
Hits 20196 20196
Misses 7425 7425
Partials 21 21
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
| run: >- | ||
| cmake -S . -B "$BUILD_DIR" | ||
| -DCMAKE_BUILD_TYPE=Release | ||
| -DUSE_CUDA=ON -DUSE_CUTENSOR=ON -DUSE_CUQUANTUM=ON | ||
| -DRUN_TESTS=ON -DBUILD_PYTHON=OFF | ||
| -DCMAKE_CUDA_ARCHITECTURES="$GPU_CUDA_ARCH" | ||
|
|
There was a problem hiding this comment.
Consider to use preset debug-openblas-cuda or add a new test preset like #1023
There was a problem hiding this comment.
Thanks. This job is intentionally a Release native build against the runner's own system-installed CUDA/cuTENSOR/cuQuantum (no conda/PyPI toolchain step), which doesn't map cleanly onto the current presets — debug-openblas-cuda is Debug+ASan, and it also keeps the build config close to what a from-source user would run on their own GPU box. I agree a dedicated CUDA test preset like #1023 is the right long-term home for these flags; moving this workflow onto it once #1023 lands would be a clean follow-up so the config lives in one place.
| CUTENSOR_ROOT: ${{ vars.CUTENSOR_ROOT }} | ||
| CUQUANTUM_ROOT: ${{ vars.CUQUANTUM_ROOT }} |
There was a problem hiding this comment.
If we can control the dependency versions like #1023 will be better. Because that will be the versions we will release.
There was a problem hiding this comment.
|
There are conflicts that need to be resolved. |
b2329d3 to
9d4c9f8
Compare
IvanaGyro
left a comment
IvanaGyro
left a comment
There was a problem hiding this comment.
Will this workflow consider to use predefined preset?
Problem: there is still no GPU CI (the origin issue #538). ci-cmake_tests.yml is CPU-only, and the existing gpu-smoke-test (main.yml) targeted a non-existent `v100` self-hosted runner and shelled out to Docker (not installed on the runner), so it never ran a single job. Separately, gpu_test_main could not even compile after #1004: its host-compiled .cpp TUs include Type.hpp, which pulls <cuda/std/complex>, but cytnx adds the CUDA toolkit + CCCL include dirs PRIVATE, so they never propagated to the test target. Fix: - tests/gpu/CMakeLists.txt: give gpu_test_main the CUDA toolkit + CCCL include dirs (fixes "fatal error: cuda/std/complex: No such file or directory"), tag its cases with the `gpu` ctest label, and enumerate them DISCOVERY_MODE PRE_TEST so a CUDA-enabled build still links on a driver-less machine. - CytnxBKNDCMakeLists.cmake: expose the detected CCCL dir as CYTNX_CCCL_INCLUDE_DIR so the test target reuses cytnx's single-source detection instead of duplicating it. - CMakePresets.json: add a `gpu-only` test preset (filters on the `gpu` label). - .github/workflows/ci-gpu_tests.yml: native build on the self-hosted GPU runner using the box's CUDA + cuTENSOR + cuQuantum (roots via repo vars), building gpu_test_main and running `ctest -L gpu`. Manual dispatch plus same-repo PRs only; fork PRs never execute on self-hosted hardware. - Remove the dead gpu-smoke-test workflow (main.yml). Testing: on the runner box (2x RTX 4070 Ti SUPER, sm_89, CUDA 13.0, cuTENSOR 2.0, cuQuantum) configure + `cmake --build --target gpu_test_main` succeed, `ctest -L gpu` discovers 788 tests, and AccessorTest passes 15/15 on-GPU. Toward #538. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
BlockUniTensorTest.gpu_Trace read the rank-0 scalar from UT_diag.Trace(0, 1)
via at({0}), which #1026 (rank-0 UniTensor semantics) now rejects with
"rank-0 scalar expects no locator indices". The CPU Trace test was updated to
at({}) in #1026; the GPU test was missed because GPU tests don't run in CI.
Mirror the CPU fix. Surfaced by this PR's new GPU CI on its first run.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
9d4c9f8 to
5afdb51
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5afdb51505
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| on: | ||
| workflow_dispatch: | ||
| pull_request: | ||
| branches: | ||
| - master |
There was a problem hiding this comment.
Because the job deliberately skips fork pull requests and this trigger has no push entry, code merged from a fork into master is never exercised by this GPU workflow unless someone manually dispatches it. Add a push trigger for master; that runs only the trusted merged revision while preserving the restriction against executing fork code on the self-hosted runner.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — addressed in e9e692c by adding a push: trigger for master. Code merged from a fork PR (which the pull_request run deliberately skips) now runs the GPU suite on the trusted merged revision. A push event is never a fork, so the existing job if guard runs it unconditionally while still skipping fork pull_request events on the self-hosted runner.
Address review feedback on #1044: - tests/gpu/CMakeLists.txt: wrap the CUDAToolkit_INCLUDE_DIRS include in an if() guard so an empty/undefined variable (e.g. CUDA in standard system paths) doesn't expand to an empty target_include_directories() call and fail configuration (gemini-code-assist). - ci-gpu_tests.yml: add a push trigger for master so code merged from a fork PR -- which the pull_request run deliberately skips -- is still exercised on the trusted merged revision. A push event is never a fork, so the existing job `if` guard always runs it (codex). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| # gpu_test_main's .cpp TUs are host-compiled (g++) but include cytnx public | ||
| # headers (Type.hpp) that pull <cuda/std/complex>. cytnx adds the CUDA toolkit | ||
| # and CCCL include dirs PRIVATEly, so they do not propagate here through | ||
| # linking; add them explicitly, otherwise the host compile fails with | ||
| # "fatal error: cuda/std/complex: No such file or directory". | ||
| if(CUDAToolkit_INCLUDE_DIRS) | ||
| target_include_directories(gpu_test_main PRIVATE ${CUDAToolkit_INCLUDE_DIRS}) | ||
| endif() | ||
| if(CYTNX_CCCL_INCLUDE_DIR) | ||
| target_include_directories(gpu_test_main PRIVATE ${CYTNX_CCCL_INCLUDE_DIR}) | ||
| endif() |
There was a problem hiding this comment.
If the public headers need <cuda/std/complex>, the include path of that header should be public for cytnx.
2 participants
Problem
No GPU CI exists (origin issue #538).
ci-cmake_tests.ymlis CPU-only; the oldgpu-smoke-test(main.yml) targeted a non-existentv100runner and shelled out to Docker (not installed), so it never ran a single job. And after #1004gpu_test_maindidn't even compile: its host-compiled.cppTUs includeType.hpp→<cuda/std/complex>, butcytnxadds the CUDA toolkit + CCCL includes PRIVATE, so they never reached the test target (fatal error: cuda/std/complex: No such file or directory).Fix
tests/gpu/CMakeLists.txt— givegpu_test_mainthe CUDA toolkit + CCCL include dirs (the compile fix); tag its cases with thegpuctest label; enumerateDISCOVERY_MODE PRE_TESTso a CUDA build links even on a driver-less machine.CytnxBKNDCMakeLists.cmake— expose the detected CCCL dir asCYTNX_CCCL_INCLUDE_DIR(single source of truth, reused by the test target rather than re-detected).CMakePresets.json— add agpu-onlytest preset (filters on thegpulabel)..github/workflows/ci-gpu_tests.yml— native build on the self-hosted GPU runner using the box's CUDA / cuTENSOR / cuQuantum (roots come from repovars.CUTENSOR_ROOT/vars.CUQUANTUM_ROOT, already configured); buildsgpu_test_mainand runsctest -L gpu. Manual dispatch + same-repo PRs only — fork PRs never execute on self-hosted hardware.gpu-smoke-testworkflow (main.yml).The
gpulabel +PRE_TESTgroundwork is adapted from the draft #1023 (which targeted a GitHub-hosted runner and, being unrun, still carried the CCCL compile blocker). This supersedes #1023's direction — suggest closing it once this lands.Testing
On the runner box (2× RTX 4070 Ti SUPER, sm_89, CUDA 13.0, cuTENSOR 2.0, cuQuantum):
cmakeconfigure +cmake --build --target gpu_test_mainsucceed — CUDA lib + test binary, nocuda/std/complexerror.ctest -L gpudiscovers 788 GPU-labeled tests;AccessorTestpasses 15/15 on-GPU.pull_requestrun exercises the workflow on the self-hosted runner end-to-end.Known runner-env caveat (not a blocker)
The runner's
cuquantumconda env is CUDA-11-built, so linking warnslibcublas.so.11 … may conflict with libcublas.so.13. Core tensor tests are unaffected; updating that env to a CUDA-13 cuQuantum build clears the warning.Toward #538.
🤖 Generated with Claude Code