cuda.core.system: Better checks for when we expect APIs to be unsupported

[mdboom]

Prior to this PR, unit tests in cuda.core.system would just always skip the test if an API returned an NotSupportedError. This meant that a test could be skipped even when the documentation suggests that it should in fact work. This PR makes it so such a skip would only be acceptable if the API is documented as not being supported for a given architecture.

There is some nuance, though. Some of these APIs are still failing as not supported even when the documentation says they should be. This may be because some other aspect of the device (not just the architecture) makes it unsupported. Unfortunately, the NVML headers don't offer any clues there, but at least now we know where all of these cases are (because they are noted with unsupported_before(device, None), and we can follow up by filing documentation bugs with NVML and refining the criteria by which tests are skipped even further.

This also removes the behavior of some tests where it would collect different skip reasons for each device and report all of them. This added a lot of complexity for little benefit -- it is probably very rare that a system would have two devices of different architectures, especially on systems that we would use for testing -- so I just took all of that out to simplify things.

[copy-pr-bot]

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.

[Copilot]

Pull request overview

This PR improves test coverage for NVML APIs in cuda.core.system by implementing better checks for when APIs are expected to be unsupported based on device architecture. The changes replace blanket exception catching with architecture-aware validation using a new unsupported_before context manager.

Changes:

• Introduced unsupported_before context manager to conditionally skip or assert on API support based on device architecture
• Renamed DeviceArchitecture class to DeviceArch to align with the underlying NVML enum
• Simplified test logic by removing complex skip reason collection and replacing it with the new context manager pattern

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 35 comments.

Show a summary per file

File	Description
cuda_core/tests/system/conftest.py	Added unsupported_before context manager for architecture-aware test skipping
cuda_bindings/tests/nvml/conftest.py	Added unsupported_before context manager for NVML bindings tests
cuda_core/tests/system/test_system_device.py	Refactored tests to use unsupported_before and updated to use DeviceArch
cuda_bindings/tests/nvml/test_pynvml.py	Updated tests to use unsupported_before
cuda_bindings/tests/nvml/test_gpu.py	Updated tests to use unsupported_before
cuda_bindings/tests/nvml/test_compute_mode.py	Updated tests to use unsupported_before
cuda_core/cuda/core/system/_device.pyx	Renamed DeviceArchitecture to DeviceArch and moved functionality to use NVML enum directly
cuda_core/docs/source/api.rst	Updated documentation to reference DeviceArch instead of DeviceArchitecture

Comments suppressed due to low confidence (1)

cuda_core/cuda/core/system/_device.pyx:1050

• The docstring still references the old class name "DeviceArchitecture" instead of the new "DeviceArch". The documentation should be updated to match the renamed class.

 """
return DeviceEvents(self._handle, events)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[mdboom]

/ok to test

[mdboom]

/ok to test

[mdboom]

/ok to test

[mdboom]

/ok to test

[mdboom]

/ok to test

[rwgk]

The below came from Cursor (after discarding my cuda.bindings.tests.helpers idea I posted offline).

The idea is that you can give this to Cursor on your end to play with the options.

---

Suggestion: Deduplicating unsupported_before

I noticed we now have two nearly identical unsupported_before context managers:

• cuda_bindings/tests/nvml/conftest.py
• cuda_core/tests/system/conftest.py

The logic is the same, but they use different API surfaces:

Aspect	nvml conftest	cuda.core conftest
Get arch	nvml.device_get_architecture(device)	device.arch
Get name	nvml.device_get_name(device)	device.name
Arch enum	nvml.DeviceArch	system.DeviceArch
Exception	nvml.NotSupportedError	system.NotSupportedError

Options to consider

Option 1: Factory pattern in cuda_python_test_helpers (recommended)

Keep cuda_python_test_helpers CUDA-agnostic but share the logic via dependency injection:

# cuda_python_test_helpers/__init__.pyfromcontextlibimportcontextmanagerdefmake_unsupported_before(*, get_arch, get_name, arch_enum, not_supported_error):
"""Factory to create an unsupported_before context manager with injected dependencies."""@contextmanagerdefunsupported_before(device, expected_device_arch):
device_arch=get_arch(device)
ifisinstance(expected_device_arch, arch_enum):
expected_device_arch_int=int(expected_device_arch)
elifexpected_device_arch=="FERMI":
expected_device_arch_int=1else:
expected_device_arch_int=0if (
expected_device_archisNoneorexpected_device_arch=="HAS_INFOROM"ordevice_arch==arch_enum.UNKNOWN
):
try:
yieldexceptnot_supported_error:
importpytestpytest.skip(
f"Unsupported call for device architecture {arch_enum(device_arch).name} "f"on device '{get_name(device)}'"
)
elifint(device_arch) <expected_device_arch_int:
importpytestwithpytest.raises(not_supported_error):
yieldpytest.skip(f"Unsupported before {expected_device_arch.name}, got {get_name(device)}")
else:
yieldreturnunsupported_before

Then in each conftest:

# cuda_bindings/tests/nvml/conftest.pyfromcuda_python_test_helpersimportmake_unsupported_beforefromcuda.bindingsimport_nvmlasnvmlunsupported_before=make_unsupported_before(
get_arch=nvml.device_get_architecture,
get_name=nvml.device_get_name,
arch_enum=nvml.DeviceArch,
not_supported_error=nvml.NotSupportedError,
)

# cuda_core/tests/system/conftest.pyfromcuda_python_test_helpersimportmake_unsupported_beforefromcuda.coreimportsystemunsupported_before=make_unsupported_before(
get_arch=lambdad: d.arch,
get_name=lambdad: d.name,
arch_enum=system.DeviceArch,
not_supported_error=system.NotSupportedError,
)

Pros: Single source of truth for the logic, cuda_python_test_helpers stays CUDA-agnostic.
Cons: Slightly more indirection.

Option 2: Let cuda_python_test_helpers depend on cuda-bindings

Provide a single nvml-based implementation. cuda.core tests would extract device handles.

Pros: Simpler API.
Cons: Reverses the direction of commit 6afdd5c; ties test helpers to CUDA packages.

Option 3: Accept the duplication

~40 lines duplicated in 2 files. The implementations use different abstraction layers, so some might argue they're legitimately different.

Pros: No new abstractions.
Cons: Bug fixes / enhancements need to be applied twice.

---

I'd lean toward Option 1 since it keeps the CUDA-agnostic design while eliminating the duplication. Happy to pair on this if you want to explore it further.

[mdboom]

As is often the case with these AI tools, Cursor's suggestion is way more complex than it needs to be.

I think we can:

1. Create a cuda.bindings._test_helpers package to put these sort of "shared" things into. We don't need to include the entire test suite in the cuda.bindings package in order to do this. (This probably requires a little extra design approval before we commit to it).

2. The cuda.core.system version of this helper can just delegate to the cuda.bindings.nvml one by passing along the handle.

[mdboom]

/ok to test

[mdboom]

/ok to test

[mdboom]

/ok to test

[rwgk]

The new _test_helpers approach looks great to me.

it is probably very rare that a system would have two devices of different architectures, especially on systems that we would use for testing -- so I just took all of that out to simplify things.

Just logging a concern: While this sounds reasonable today, it could be very surprising in the future, because a skip for one device may mask issues with another, and may silently reduce test coverage in a way that's easy to miss. Skips generally look harmless, while these skips are maybe not. Ideally the potentially failure-masking skips should come with a warning; in this case, only if there are more devices left to loop over, which isn't so easy to get right I guess.

[rparolin]

Consider: adding inline code comments around what the pattern of wrapping a try with exception handling is doing.

[rparolin]

lgtm, a couple minor comments to consider but are non-blocking

[mdboom]

Just logging a concern: While this sounds reasonable today, it could be very surprising in the future, because a skip for one device may mask issues with another, and may silently reduce test coverage in a way that's easy to miss. Skips generally look harmless, while these skips are maybe not. Ideally the potentially failure-masking skips should come with a warning; in this case, only if there are more devices left to loop over, which isn't so easy to get right I guess.

Thinking about this more, I think we can write a test for this case, so at least it might offer a clue to our future selves. I'll add it here.

[mdboom]

/ok to test

[mdboom]

/ok to test

[github-actions]

Doc Preview CI
Preview removed because the pull request was closed or merged.

[leofang]

@mdboom@rwgk@rparolin Sorry I did not notice this until now. Because neither this PR not #1585 was backported, we now have a huge discrepancy between main and 12.9.x. Here's a CI failure that (partially) blocked us from cutting a release today:
https://github.com/NVIDIA/cuda-python/actions/runs/22879760518/job/66380294102?pr=1745#step:29:167

E ImportError: cannot import name 'hardware_supports_nvml' from 'cuda.bindings._test_helpers.arch_check' (/opt/hostedtoolcache/Python/3.10.20/x64/lib/python3.10/site-packages/cuda/bindings/_test_helpers/arch_check.py)

Any files touched under cuda_bindings/ need to be backported.