AcademySoftwareFoundation / AcademySoftwareFoundation/openvdb
NanoVDB: please run the CUDA unit tests on a GPU in CI
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 3.4k
- Forks
- 777
- Avg merge
- 3d 9h
- Merged PRs (30d)
- 34
Description
Request
Please add GPU-backed execution of the NanoVDB CUDA unit tests to CI. Today they are compiled and then skipped, so no device-side runtime behaviour in NanoVDB is verified before merge.
Current state
All three ctest invocations in .github/workflows/nanovdb.yml carry the same filter:
run: cd build && sudo ctest -V -E ".*cuda.*|.*mgpu.*"
Of the 7 registered tests, 5 never execute:
| test | runs in CI |
|---|---|
nanovdb_unit_test |
yes |
nanovdb_openvdb_unit_test |
yes |
nanovdb_cuda_unit_test |
no |
nanovdb_mgpu_unit_test |
no |
nanovdb_cuda_memory_resource_unit_test |
no |
nanovdb_cuda_util_unit_test |
no |
nanovdb_cuda_util_sync_unit_test |
no |
#2251 adds a sixth (nanovdb_cuda_buffer_unit_test), which will also be skipped.
The exclusion is correct given the runners — ubuntu-22.04-8c-32g-300h and windows-2022-8c-32g-300h have no GPU, so these tests would fail with CUDA error 35. The Linux job does configure -DNANOVDB_USE_CUDA=ON -DCMAKE_CUDA_ARCHITECTURES="80", so compilation of device code is covered. Runtime behaviour is not.
Why this matters now
Every device-side guarantee in NanoVDB is currently unverified by CI. Recent work that CI cannot check: stream-ordered allocation and stream-retained frees (#2231), memory-pool capability detection and its fallback (#2256), buffer resize/copy/move semantics and graph-capturability (#2251), and the distributed merge-path bounds fix (#2248). Regressions in any of these reach master green.
There are second-order effects too. There is no place to put a performance regression guard — relevant for #2260, where the ReadAccessor caching fix is invisible to value assertions and observable only as a timing difference, so a benchmark is the only possible guard. And correctness questions about GPU behaviour cannot be settled by CI at all; they currently require a maintainer to run by hand on their own hardware and report back, which is how the last several device-side issues were resolved.
What would satisfy this
In rough order of preference, acknowledging that GPU runner time costs money:
- A single-GPU runner covering the CUDA tests. This covers 4 of the 5 skipped tests; only
nanovdb_mgpu_unit_testneeds more than one device. Nightly rather than per-PR would bound the cost while still catching regressions within a day. - A manually-triggered
workflow_dispatchGPU job, so a maintainer can request a device-side run on a PR that warrants one without paying for it on every push. - At minimum, document the gap — state in the workflow and in the NanoVDB developer docs that CUDA tests are build-only in CI and must be run locally before merging device-side changes.
Whichever route is taken, the cuda/mgpu ctest naming convention should be written down: it is load-bearing and currently undocumented, so a GPU-requiring test named without one of those substrings will silently run GPU-less in CI and fail.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with .github/workflows/nanovdb.yml and inspect the three ctest commands, runner definitions, and the existing CMAKE_CUDA_ARCHITECTURES configuration. Compare the available GPU execution options in the request, then check the NanoVDB developer documentation locations. Done means the chosen GPU or documentation path is implemented and the cuda/mgpu test naming convention is recorded.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cmake, cpp, github-actions
- Domain
- ci-cd, infrastructure, testing-qa
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100