Skip to content

[cuda] fix: hide nvml-only unstartable gpu listings - #135

Merged
Wangmerlyn merged 2 commits into
mainfrom
codex/cuda-nvml-startable-listing
Jun 28, 2026
Merged

[cuda] fix: hide nvml-only unstartable gpu listings#135
Wangmerlyn merged 2 commits into
mainfrom
codex/cuda-nvml-startable-listing

Conversation

@Wangmerlyn

@Wangmerlyn Wangmerlyn commented Jun 28, 2026

Copy link
Copy Markdown
Owner

Summary

  • Gate CUDA NVML list_gpus records on Torch CUDA availability, matching visible count, and successful set_device() probes for each visible ordinal.
  • Fall back from untrusted NVML to Torch CUDA, and avoid non-HIP ROCm SMI fallback that could mislabel CUDA ordinals.
  • Update AGENTS, README, architecture, CLI/MCP/Python/API docs, and add a plan with RED/GREEN evidence.

Test Plan

  • PYTHONPATH=$PWD/src pytest tests/utilities/test_gpu_info.py -q
  • PYTHONPATH=$PWD/src pytest tests/mcp tests/utilities/test_gpu_info.py -q
  • PYTHONPATH=$PWD/src pytest tests/global_controller tests/utilities/test_gpu_info.py -q
  • PYTHONPATH=$PWD/src pytest tests -q
  • PYTHONPATH=$PWD/src mkdocs build
  • pre-commit run --all-files
  • git diff --check

Local Review

  • CUDA/startability review found fallback and set_device gaps; fixed with RED regressions.
  • Tests/docs/hygiene review found set_device gap and architecture doc gap; fixed.
  • Final follow-up review: ready to merge, no remaining issues.

Summary by CodeRabbit

  • Bug Fixes

    • GPU listings now only show CUDA devices when they can actually be started by the runtime, preventing unusable entries from appearing.
    • Hidden NVML-only devices are no longer advertised as selectable GPU targets.
  • Documentation

    • Updated CLI, API, architecture, Python, and reference docs to clarify CUDA/NVML visibility rules and startup behavior.
    • Added a new planning note describing the aligned GPU listing approach.
  • Tests

    • Expanded GPU visibility test coverage for CUDA availability, masking, fallback, and startup edge cases.

@coderabbitai

coderabbitai Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@Wangmerlyn, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 53 minutes and 23 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b37a03e2-067c-416b-b7e4-fcce9f117c29

📥 Commits

Reviewing files that changed from the base of the PR and between 929bd2c and 8c6165f.

📒 Files selected for processing (1)
  • docs/plans/cuda-nvml-startable-listing.md
📝 Walkthrough

Walkthrough

Gates NVML GPU enumeration on Torch CUDA startability: two new helper functions check Torch's visible device count and probe each ordinal via set_device; if checks fail, _query_nvml returns empty. Non-HIP get_gpu_info no longer falls back to ROCm SMI. Tests are refactored around a shared CUDA mock and expanded with new startability/fallback cases. All documentation is updated accordingly.

Changes

CUDA NVML Startable Listing Gate

Layer / File(s) Summary
Torch startability helpers and NVML gate
src/keep_gpu/utilities/gpu_info.py
Adds _torch_cuda_visible_count() and _torch_cuda_visible_ordinals_startable(), wires them as an early-exit gate in _query_nvml, updates get_gpu_info docstring and non-HIP fallback order to go NVML → Torch → MPS, skipping ROCm SMI.
Test mock infrastructure and new/refactored tests
tests/utilities/test_gpu_info.py
Introduces DummyTorchCudaCUDA and _install_cuda_gpu_info_mocks; refactors all existing NVML tests to the shared helper; adds 7 new tests covering torch-unavailable, count-zero, probe-failure, set_device-failure, mask-mismatch fallback, ROCm SMI non-fallback, and version-attribute-absent paths.
Documentation and plan
AGENTS.md, README.md, docs/concepts/architecture.md, docs/guides/..., docs/reference/..., docs/plans/cuda-nvml-startable-listing.md
Updates all user-facing and reference docs to state that NVML devices are only exposed when Torch CUDA can start the same visible ordinal set; adds the plan document with requirements checklist and verification log.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • Wangmerlyn/KeepGPU#59: Modifies the same gpu_info.py / get_gpu_info implementation and NVML-related tests that this PR's startability gate builds on.
  • Wangmerlyn/KeepGPU#103: Directly touches get_gpu_info backend precedence between CUDA NVML and ROCm/HIP, the same control-flow area this PR rewrites for non-HIP builds.

Poem

🐇 Hop hop, said the rabbit with glee,
"NVML won't list what Torch cannot see!
We probe every ordinal, set_device and all,
If Torch says 'no start,' we return nothing at all.
Only true, startable GPUs — that's the decree!" 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 2.63% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the PR’s main change: hiding NVML-only CUDA listings that Torch cannot start.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/cuda-nvml-startable-listing

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request ensures that CUDA NVML records are only returned when Torch CUDA can address the same visible ordinal set, preventing the advertisement of NVML-only devices that cannot be started. It introduces validation checks in gpu_info.py to verify Torch CUDA availability, matching visible counts, and startable ordinals, and removes the fallback to ROCm SMI on non-HIP builds. Comprehensive unit tests and documentation updates have been added to support these changes. No review comments were provided, so there is no feedback to address.

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/plans/cuda-nvml-startable-listing.md`:
- Line 41: The checklist state is inconsistent with the later verification log,
so update the plan to reflect one clear status. In the markdown plan, either
mark the local subagent code review item as completed or rewrite the follow-up
entries in the verification log so they match the same unresolved/completed
state. Keep the wording aligned around the review status in the checklist and
the later log entries so the plan remains consistent.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 00f5dc82-1433-48ae-91ab-909be08c0bf3

📥 Commits

Reviewing files that changed from the base of the PR and between 8f582f5 and 929bd2c.

📒 Files selected for processing (11)
  • AGENTS.md
  • README.md
  • docs/concepts/architecture.md
  • docs/guides/cli.md
  • docs/guides/mcp.md
  • docs/guides/python.md
  • docs/plans/cuda-nvml-startable-listing.md
  • docs/reference/api.md
  • docs/reference/cli.md
  • src/keep_gpu/utilities/gpu_info.py
  • tests/utilities/test_gpu_info.py

Comment thread docs/plans/cuda-nvml-startable-listing.md Outdated
@Wangmerlyn
Wangmerlyn merged commit a6d2990 into main Jun 28, 2026
5 checks passed
@Wangmerlyn
Wangmerlyn deleted the codex/cuda-nvml-startable-listing branch June 28, 2026 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant