cuda.core.system: Add device attributes, and other simple device-related APIs #1447
Conversation
mdboom
commented
Jan 9, 2026
mdboom
commented
- New or existing tests cover these changes.
- The documentation is up to date with these changes.
|
Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
There was a problem hiding this comment.
Pull request overview
This PR adds device-related APIs to the cuda.core.system module, expanding the device metadata and attribute capabilities. The changes introduce new properties for device information and support for additional device lookup methods via PCI bus ID.
Key changes:
- Added DeviceAttributes class exposing various device hardware attributes (multiprocessor count, memory size, engine counts, etc.)
- Added new Device properties: brand, index, attributes, is_c2c_mode_enabled, and persistence_mode_enabled (with setter)
- Enhanced Device constructor to support PCI bus ID lookup in addition to existing index and UUID methods
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| cuda_core/tests/system/test_system_device.py | Added comprehensive tests for new device properties (brand, PCI bus ID lookup, attributes, C2C mode, persistence mode); removed obsolete test_device_index_handle |
| cuda_core/docs/source/api.rst | Updated API documentation to include new system module exports (get_driver_branch, BrandType, DeviceAttributes) with reorganized ordering |
| cuda_core/cuda/core/system/_device.pyx | Implemented DeviceAttributes class, added BrandType export, extended Device with new properties and PCI bus ID constructor parameter, added persistence mode setter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @property | ||
| def memory_size_mb(self) -> int: | ||
| """ | ||
| Device memory size in MiB |
There was a problem hiding this comment.
The property name is memory_size_mb which suggests megabytes (MB), but the docstring says "MiB" which is mebibytes. These are different units: 1 MB = 1,000,000 bytes, while 1 MiB = 1,048,576 bytes. The naming should be consistent with the actual unit returned by the underlying NVML API to avoid confusion.
| Device memory size in MiB | |
| Device memory size in MB |
There was a problem hiding this comment.
This would be a deviation from the naming in NVML itself. What do you think, @leofang?
There was a problem hiding this comment.
Is it difficult to push a change to NVML? To correct the function so we can be consistent.
There was a problem hiding this comment.
It would be a breaking API change to NVML. We could ask them to add a second API and deprecate the old one, I suppose. But I suspect this is a pretty common inconsistency across CUDA APIs (MB vs. MiB) and unlikely to be worth the effort. (But I'm assuming lots there).
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
/ok to test |
This comment has been minimized.
This comment has been minimized.
|
/ok to test |
|
/ok to test |
|
/ok to test |
|
2 participants
