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_test needs more than one device. Nightly rather than per-PR would bound the cost while still catching regressions within a day.
- A manually-triggered
workflow_dispatch GPU 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.
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
ctestinvocations in.github/workflows/nanovdb.ymlcarry the same filter:Of the 7 registered tests, 5 never execute:
nanovdb_unit_testnanovdb_openvdb_unit_testnanovdb_cuda_unit_testnanovdb_mgpu_unit_testnanovdb_cuda_memory_resource_unit_testnanovdb_cuda_util_unit_testnanovdb_cuda_util_sync_unit_test#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-300handwindows-2022-8c-32g-300hhave 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
mastergreen.There are second-order effects too. There is no place to put a performance regression guard — relevant for #2260, where the
ReadAccessorcaching 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:
nanovdb_mgpu_unit_testneeds more than one device. Nightly rather than per-PR would bound the cost while still catching regressions within a day.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.Whichever route is taken, the
cuda/mgpuctest 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.