You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This pull request implements auto-detection for Intel GPUs (such as Intel Arc or Data Center GPUs) using xpu-smi.
Motivation & Context
Intel GPUs were previously absent from the hardware discovery pipeline, which only checked for NVIDIA (nvidia-smi), AMD (rocm-smi), and Apple Silicon (sysctl). By adding xpu-smi to the discovery flow, local hardware limits can be detected dynamically when using the --gpu auto flag, enabling correct mapping to the Intel configurations in KNOWN_GPUS.
Type of Change
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Refactoring (no functional changes, no api changes)
Documentation update
How Has This Been Tested?
The changes were validated using unit tests that mock the output of xpu-smi discovery to verify that both single-GPU and multi-GPU setups parse correctly and aggregate their VRAM capacities.
Unit tests
Integration tests
Manual testing
Checklist
My code follows the code style of this project.
My commit messages follow the Conventional Commits format, are lowercase, imperative, and specific.
I have updated the documentation accordingly (if applicable).
I have added tests to cover my changes.
All new and existing tests passed.
Summary by CodeRabbit
New Features
Improved local GPU/accelerator detection with updated priority order (NVIDIA → AMD → Intel XPU via xpu-smi → Apple Silicon), including per-device Intel naming and aggregated VRAM totals.
Bug Fixes
Enhanced xpu-smi parsing and unit conversion for “Memory Physical Size” (GiB/GB/KiB/KB and byte variants) to produce consistent VRAM values.
Improved resilience to malformed or mismatched xpu-smi output, reliably falling back to the default “Unknown” result.
Disabled the previous Apple Silicon unified-memory heuristic so Apple detection no longer succeeds.
Tests
Expanded coverage for Intel XPU discovery, multi-device aggregation, unit conversion, and fallback behavior.
It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.
Use the following commands to manage reviews:
@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.
Use the checkboxes below for quick actions:
▶️ Resume reviews
🔍 Trigger review
Walkthrough
detect_local_gpu() now checks NVIDIA, AMD, Intel, and Apple helpers in order, parses Intel xpu-smi discovery output with unit normalization, and returns no Apple result. Tests add single-device, multi-device, conversion, malformed-output, and fallback coverage.
Changes
Intel XPU GPU detection
Layer / File(s)
Summary
GPU helper chain and Intel parsing src/modelinfo/hardware.py
detect_local_gpu() calls internal helpers in sequence, the Intel helper parses xpu-smi discovery device names and memory sizes across supported units, and Apple detection now returns no result.
Intel XPU detection tests tests/test_hardware.py
Tests cover single-device and multi-device xpu-smi discovery parsing, unit conversion across reported memory formats, malformed Intel output fallback, and the Apple fallback condition when xpu-smi is unavailable.
Sequence Diagram(s)
sequenceDiagram
participant hardware.detect_local_gpu() as DetectLocalGpu
participant _detect_nvidia_gpu() as NvidiaHelper
participant _detect_amd_gpu() as AmdHelper
participant _detect_intel_gpu() as IntelHelper
participant _detect_apple_gpu() as AppleHelper
participant xpu-smi discovery as XpuSmi
DetectLocalGpu->>NvidiaHelper: try NVIDIA detection
DetectLocalGpu->>AmdHelper: try AMD detection
DetectLocalGpu->>IntelHelper: run xpu-smi discovery
IntelHelper->>XpuSmi: execute discovery
XpuSmi-->>IntelHelper: device name and memory physical size lines
IntelHelper-->>DetectLocalGpu: Intel label, VRAM, GPU count
DetectLocalGpu->>AppleHelper: try Apple Silicon fallback
Loading
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
pipe1os/modelinfo-cli#25: Adds related detect_local_gpu() fallback and parsing coverage around mocked hardware discovery output, including the Apple fallback path.
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name
Status
Explanation
Resolution
Docstring Coverage
⚠️ Warning
Docstring coverage is 5.00% 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
Title check
✅ Passed
The title clearly and concisely summarizes the main change: Intel GPU auto-detection via xpu-smi.
Description check
✅ Passed
The description matches the template with summary, motivation, change type, testing, and checklist filled in.
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 feature/intel-gpu-detection
Comment @coderabbitai help to get the list of available commands.
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
The PR is currently not up to standards according to Codacy analysis. While it successfully implements Intel GPU auto-detection via xpu-smi, it significantly increases the technical debt in src/modelinfo/hardware.py, which has reached a cyclomatic complexity of 24.
A critical logic issue was identified in the parsing routine: if GPU names are detected but memory parsing fails (leaving the total at 0.0), the function will return an invalid result instead of falling back to other detection methods. Additionally, while the implementation provides unit conversion for various memory formats (GiB, KiB, B), these paths are currently untested. These issues, alongside the high complexity of the 'God Function' for hardware detection, should be addressed before merging.
About this PR
The unit conversion logic for non-MiB units (GiB, KiB, and Bytes) is currently untested. Given the critical nature of hardware reporting, please add unit tests covering these specific unit strings to ensure the math and regex parsing behave as expected.
1 comment outside of the diffsrc/modelinfo/hardware.py
line 160 🟡 MEDIUM RISK
Suggestion: The detect_local_gpu function is becoming a 'God Function' for hardware detection. Refactoring this into provider-specific helpers (e.g., _detect_intel_gpu) would improve maintainability and allow for easier testing of individual detection paths.
Try running the following prompt in your IDE agent:
Refactor the detect_local_gpu function in src/modelinfo/hardware.py by extracting the specific detection logic for NVIDIA (lines 161-191), AMD (lines 193-219), Intel (lines 221-264), and Apple (lines 266-280) into separate private functions. Each private function should return a tuple of (name, vram, count) or None if the respective hardware is not detected.
Test suggestions
Detection of a single Intel Arc GPU with MiB to GiB VRAM conversion
Detection and aggregation of multiple Intel Data Center GPUs
Verification of unit conversion logic for GiB, KiB, and Byte inputs
Fallback to Apple Silicon detection logic when xpu-smi command fails
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verification of unit conversion logic for GiB, KiB, and Byte inputs
The reason will be displayed to describe this comment to others. Learn more.
🟡 MEDIUM RISK
Suggestion: The function should only return if both GPU names and a non-zero memory value were successfully parsed. This ensures that a failed parse correctly falls back to subsequent detection methods or the final 'Unknown' default.
Try running the following prompt in your coding agent:
Refactor the xpu-smi parsing logic in detect_local_gpu to be more robust. Ensure the function only returns if it successfully parsed both a name and a non-zero memory value, allowing fallback to continue if parsing fails. Also, simplify the unit parsing by combining the regex searches.
The reason will be displayed to describe this comment to others. Learn more.
⚪ LOW RISK
Suggestion: Simplify the extraction of value and unit using a single regex with groups. This makes the parsing logic cleaner and more explicit.
match=re.search(r"([\d\.]+)\s*([a-zA-Z]*)", size_str)
ifmatch:
val=float(match.group(1))
unit=match.group(2).lower()
ifunitin ("gib", "gb"):
val*=1024.0elifunitin ("kib", "kb"):
val/=1024.0elifunit=="b":
val/= (1024.0*1024.0)
# Note: mib/mb units require no conversion as the accumulator is in MiBtotal_mib+=val
The reason will be displayed to describe this comment to others. Learn more.
⚪ LOW RISK
Nitpick: While 'assert' is standard practice in pytest, it is being flagged by the security linter (Bandit B101). To satisfy the linter without changing the test logic, you can add a '# nosec' comment to the line.
These tests cover the happy paths, but not the case where Device Name: parses and Memory Physical Size: does not. Once the parser guard is fixed, add a test that expects fallback instead of (intel_name, 0.0, 1) so this doesn't regress silently.
🤖 Prompt for 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.
In `@tests/test_hardware.py` around lines 111 - 156, The Intel GPU detection tests
in detect_local_gpu only cover valid xpu-smi output, but not the malformed case
where Device Name parses while Memory Physical Size fails. Add a regression test
alongside test_detect_local_gpu_falls_back_to_xpu_smi and
test_detect_local_gpu_sums_multiple_intel_gpus that stubs subprocess.run for
xpu-smi discovery with an invalid memory value and asserts detect_local_gpu
falls back instead of returning an Intel name with 0.0 memory and 1 device. Use
the detect_local_gpu helper and the existing completed/fake_run pattern so the
parser guard stays covered.
🤖 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 `@src/modelinfo/hardware.py`:
- Around line 230-262: The Intel GPU detection path in the hardware parsing
logic is treating any collected `gpu_names` as a successful match even when
`Memory Physical Size:` was never parsed, which can return a bogus zero-VRAM
result. Update the parsing flow in the hardware info function so it only returns
from this branch when VRAM parsing succeeded, and otherwise falls through to the
non-Intel fallback; use the existing GPU parsing symbols like `gpu_names`,
`total_mib`, and the Intel display-name construction to locate the change.
---
Nitpick comments:
In `@tests/test_hardware.py`:
- Around line 111-156: The Intel GPU detection tests in detect_local_gpu only
cover valid xpu-smi output, but not the malformed case where Device Name parses
while Memory Physical Size fails. Add a regression test alongside
test_detect_local_gpu_falls_back_to_xpu_smi and
test_detect_local_gpu_sums_multiple_intel_gpus that stubs subprocess.run for
xpu-smi discovery with an invalid memory value and asserts detect_local_gpu
falls back instead of returning an Intel name with 0.0 memory and 1 device. Use
the detect_local_gpu helper and the existing completed/fake_run pattern so the
parser guard stays covered.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1d93d2ca-b1c4-48eb-b532-f447d6cf1c4f
📥 Commits
Reviewing files that changed from the base of the PR and between 9634347 and bbac845.
Line 259 still treats any non-zero total as success. If xpu-smi reports two Device Name: rows but only one parseable Memory Physical Size:, this returns gpu_count=2 with one GPU’s VRAM, and the CLI consumes those values for capacity analysis. Track parsed memory entries and require them to match the parsed device count.
Proposed fix
gpu_names: list[str] = []
total_mib: float = 0.0
+ parsed_memory_entries = 0
for line in result.stdout.splitlines():
lower_line = line.lower()
if "device name:" in lower_line:
@@
val /= 1024.0
elif unit == "b":
val /= (1024.0 * 1024.0)
total_mib += val
+ parsed_memory_entries += 1- if gpu_names and total_mib > 0.0:+ if gpu_names and parsed_memory_entries == len(gpu_names) and total_mib > 0.0:
gpu_count = len(gpu_names)
🤖 Prompt for 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.
In `@src/modelinfo/hardware.py` around lines 233 - 265, The Intel GPU parsing in
hardware.py still accepts a non-zero total memory even when not every parsed
Device Name has a corresponding parseable Memory Physical Size. Update the Intel
xpu-smi parsing block in the hardware detection logic to track how many memory
rows were successfully parsed and only return from the GPU branch when that
count matches the number of collected gpu_names; otherwise fall through so
incomplete GPU capacity data is not reported. Use the existing symbols
gpu_names, total_mib, and the Intel Multi-GPU return path to locate and adjust
the check.
🤖 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 `@tests/test_hardware.py`:
- Around line 170-179: Bind the loop variable used by fake_run so Ruff B023 is
satisfied: the closure in fake_run currently captures size_str from the
surrounding loop, so update fake_run to take size_str as a defaulted parameter
and keep the existing xpu-smi discovery behavior unchanged. This applies in the
test_hardware helper around fake_run and the size_str-driven stdout
construction.
---
Duplicate comments:
In `@src/modelinfo/hardware.py`:
- Around line 233-265: The Intel GPU parsing in hardware.py still accepts a
non-zero total memory even when not every parsed Device Name has a corresponding
parseable Memory Physical Size. Update the Intel xpu-smi parsing block in the
hardware detection logic to track how many memory rows were successfully parsed
and only return from the GPU branch when that count matches the number of
collected gpu_names; otherwise fall through so incomplete GPU capacity data is
not reported. Use the existing symbols gpu_names, total_mib, and the Intel
Multi-GPU return path to locate and adjust the check.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4de6be92-dbe8-4a9a-9c9e-15c83b7d36b6
📥 Commits
Reviewing files that changed from the base of the PR and between bbac845 and 3017db1.
The supplied downstream test still expects detect_local_gpu() to call sysctl hw.memsize and return the Apple unified-memory tuple after NVIDIA/AMD/Intel detection fails. Returning None here makes Apple --gpu auto fall through to ("Unknown", 8.0, 1) instead.
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/modelinfo/hardware.py` around lines 288 - 312, Restore the Apple Silicon
fallback in detect_local_gpu() so that after _detect_nvidia_gpu(),
_detect_amd_gpu(), and _detect_intel_gpu() all return None, the function still
calls _detect_apple_gpu() and returns its unified-memory tuple when available;
do not let the flow skip directly to the "Unknown" default. Locate the fix in
detect_local_gpu() and keep the Apple-specific detection path intact so Apple
--gpu auto continues to resolve via sysctl hw.memsize rather than falling
through to the generic fallback.
🤖 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.
Outside diff comments:
In `@src/modelinfo/hardware.py`:
- Around line 288-312: Restore the Apple Silicon fallback in detect_local_gpu()
so that after _detect_nvidia_gpu(), _detect_amd_gpu(), and _detect_intel_gpu()
all return None, the function still calls _detect_apple_gpu() and returns its
unified-memory tuple when available; do not let the flow skip directly to the
"Unknown" default. Locate the fix in detect_local_gpu() and keep the
Apple-specific detection path intact so Apple --gpu auto continues to resolve
via sysctl hw.memsize rather than falling through to the generic fallback.
ℹ️ Review info⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f3e66644-83ad-4ff0-bb8b-d9a8fc914a61
📥 Commits
Reviewing files that changed from the base of the PR and between 3017db1 and 40102dc.
📒 Files selected for processing (2)
src/modelinfo/hardware.py
tests/test_hardware.py
🚧 Files skipped from review as they are similar to previous changes (1)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Summary
This pull request implements auto-detection for Intel GPUs (such as Intel Arc or Data Center GPUs) using
xpu-smi.Motivation & Context
Intel GPUs were previously absent from the hardware discovery pipeline, which only checked for NVIDIA (
nvidia-smi), AMD (rocm-smi), and Apple Silicon (sysctl). By addingxpu-smito the discovery flow, local hardware limits can be detected dynamically when using the--gpu autoflag, enabling correct mapping to the Intel configurations inKNOWN_GPUS.Type of Change
How Has This Been Tested?
The changes were validated using unit tests that mock the output of
xpu-smi discoveryto verify that both single-GPU and multi-GPU setups parse correctly and aggregate their VRAM capacities.Checklist
Summary by CodeRabbit
New Features
xpu-smi→ Apple Silicon), including per-device Intel naming and aggregated VRAM totals.Bug Fixes
xpu-smiparsing and unit conversion for “Memory Physical Size” (GiB/GB/KiB/KB and byte variants) to produce consistent VRAM values.xpu-smioutput, reliably falling back to the default “Unknown” result.Tests