Enable Vulkan inference on NVIDIA runner images - #22
Conversation
|
Warning Review limit reachedNext included review available in 18 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (10)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (10)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds Vulkan runtime packages and verification to Vulkan and CUDA images. It validates NVIDIA Vulkan devices, integrates image contract checks, narrows affected backend planning, and adds tests and documentation. ChangesVulkan runner support
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to CUDA and Vulkan runner images now include Vulkan diagnostics and validate that an NVIDIA Vulkan device is available before inference. The implementation includes matching image-contract and verifier coverage, with no concrete current-head merge-blocking risk identified. Sequence Diagram(s)sequenceDiagram
participant RunnerImage
participant verify-vulkan-device
participant nvidia-smi
participant vulkaninfo
RunnerImage->>verify-vulkan-device: Start verification
verify-vulkan-device->>nvidia-smi: Enumerate NVIDIA devices
nvidia-smi-->>verify-vulkan-device: Return GPU list
verify-vulkan-device->>vulkaninfo: Collect Vulkan summary
vulkaninfo-->>verify-vulkan-device: Return device and vendor data
verify-vulkan-device-->>RunnerImage: Report success or diagnostics
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 5 files. (5 skipped: 5 unsupported.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
nvidia-smi -L exits zero on some driver builds even when it lists no device, so the exit status alone did not establish that NVML saw a GPU. Assert the enumerated output instead and cover the empty-enumeration case in the Vulkan runner suite.
22ccfe6 to
8b82897
Compare
|
@coderabbitai full review |
|
2 participants
The shared NVIDIA runner image lacked the Vulkan loader, tools, graphics capability, and a live device check, so the new product integration Vulkan smoke could not run on the existing
gpu-nvidiascale set.This adds Vulkan support to the CUDA and Vulkan image profiles while retaining CUDA, and introduces
verify-vulkan-deviceto prove the NVIDIA GPU is visible through both NVML and Vulkan. The verifier checksnvidia-smi,vulkaninfo --summary, and vendor ID0x10de; build-time validation checks the loader, tools, helper, and declared driver capabilities without assuming a GPU is present during image construction.Validation:
tests/*.test.shsuitesgit diff --checkDeployment acceptance still requires building and promoting the image, updating the
gpu-nvidiascale-set digest, retainingruntimeClassName: nvidiaandnvidia.com/gpu: 1, and runningverify-vulkan-deviceon the physical runner.Summary by CodeRabbit
New Features
Documentation
Tests