Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughWalkthroughCUDA utility functions now resolve concrete CUDA indices, use device-specific caching where required, validate CUDA-only inputs, resolve NVML bandwidth by UUID, and select default generators by index. Tests cover device switching, cache behavior, MIG UUIDs, generator selection, and CPU handling. ChangesCUDA device utilities
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant DeviceUtils
participant TorchCUDA
participant NVML
Caller->>DeviceUtils: request device-specific utility
DeviceUtils->>TorchCUDA: resolve concrete CUDA index
TorchCUDA-->>DeviceUtils: return device index or properties
DeviceUtils->>NVML: resolve UUID for bandwidth lookup
NVML-->>DeviceUtils: return bandwidth data
DeviceUtils-->>Caller: return indexed utility result
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
383d93b to
7a43a92
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
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 `@flashinfer/utils.py`:
- Around line 1450-1452: Add `@functools.cache` to _get_default_generator so
generator lookups are cached per device_index and the function exposes
cache_info() for the regression test. Ensure functools is imported or otherwise
available in the module.
🪄 Autofix
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 Plus
Run ID: 366889d3-772b-4d47-9fff-57e69960bd5e
📒 Files selected for processing (2)
flashinfer/utils.pytests/utils/test_device_utils.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/utils/test_device_utils.py
📌 Description
This PR fixes CUDA device property helpers whose cached results could be incorrectly reused across different GPUs when called with an unindexed
torch.device("cuda").The changes:
torch.cuda.current_device()before cache lookup.CUDA_VISIBLE_DEVICESremapping is handled correctly.🔍 Related Issues
N/A
🚀 Pull Request Checklist
Thank you for contributing to FlashInfer! Before we review your pull request, please make sure the following items are complete.
✅ Pre-commit Checks
pre-commitby runningpip install pre-commit(or used my preferred method).pre-commit install.pre-commit run --all-filesand fixed any reported issues.🧪 Tests
unittest, etc.).Reviewer Notes
Validation completed on a Linux CUDA environment with two NVIDIA GeForce RTX 5090 GPUs:
pre-commit run --all-files: all 14 hooks passed.pytest -q tests/utils/test_device_utils.py: 10 passed.CUDA_VISIBLE_DEVICES=1,0: passed, including correct logical-to-physical UUID mapping and separate cache entries for both devices.tests/utilsrun: 19,099 passed and 973 skipped.The remaining 57 failures and one collection error reproduce identically on
origin/main:origin/main.origin/main.array.array[int]annotation error occurs on both branches.A repository-wide collection check discovered 524,276 tests with no branch-specific collection regressions.
Reviewer focus areas:
CUDA_VISIBLE_DEVICESremapping.Summary by CodeRabbit