Skip to content

Fix the field-id enum name in the nvml test helper supports_nvlink - #2560

Open
LeSingh1 wants to merge 1 commit into
NVIDIA:mainfrom
LeSingh1:nvml-util-field-id
Open

Fix the field-id enum name in the nvml test helper supports_nvlink#2560
LeSingh1 wants to merge 1 commit into
NVIDIA:mainfrom
LeSingh1:nvml-util-field-id

Conversation

@LeSingh1

@LeSingh1 LeSingh1 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor
def supports_nvlink(device):
    fields = nvml.FieldValue(1)
    fields[0].field_id = nvml.FI.DEV_NVLINK_GET_STATE
    return nvml.device_get_field_values(device, fields)[0].nvml_return == nvml.Return.SUCCESS

There is no FI attribute on cuda.bindings.nvml. The enum is FieldId
(nvml.pyx:1229), with DEV_NVLINK_GET_STATE at nvml.pyx:1454. The sibling test uses
the correct spelling:

# tests/nvml/test_nvlink.py:19
        fields[0].field_id = nvml.FieldId.DEV_NVLINK_LINK_COUNT

So the helper raises AttributeError on its first line of real work.

Nobody has noticed because it has no callers — a repo-wide grep for supports_nvlink
finds only its own definition. Contrast util.supports_ecc in the same file, which is
called from test_page_retirement.py:35,61. This is a guard that cannot fire because it
is never invoked, and would crash if it were.

Why fix rather than delete

Deleting the dead helper is the other reasonable option, and I am happy to switch if you
prefer it. I went with the fix because supports_nvlink sits next to supports_ecc and
is_vgpu as part of an obviously intended capability-probe set, and because a test now
keeps it honest instead of letting it rot again.

Test

New cuda_bindings/tests/nvml/test_util.py. It stubs nvml.device_get_field_values, so
the helper runs without an NVLink-capable device, and asserts both that it returns True
and that it queried FieldId.DEV_NVLINK_GET_STATE. It fails on main with
AttributeError: module 'cuda.bindings.nvml' has no attribute 'FI'.

The test uses the package-relative from . import util that test_gpu.py and
test_page_retirement.py already use, and inherits the hardware_supports_nvml() gate in
tests/nvml/__init__.py.

ruff check and ruff format --check are clean.

    def supports_nvlink(device):
        fields = nvml.FieldValue(1)
        fields[0].field_id = nvml.FI.DEV_NVLINK_GET_STATE

There is no `FI` attribute on cuda.bindings.nvml. The enum is `FieldId`
(nvml.pyx:1229), with DEV_NVLINK_GET_STATE at nvml.pyx:1454, and the sibling
test uses the correct spelling: test_nvlink.py:19 does
`fields[0].field_id = nvml.FieldId.DEV_NVLINK_LINK_COUNT`.

So the helper raises AttributeError on its first line of real work. Nobody
has noticed because it has no callers -- a repo-wide grep for
`supports_nvlink` finds only its own definition. Contrast util.supports_ecc,
which is called from test_page_retirement.py.

Adds tests/nvml/test_util.py, which stubs nvml.device_get_field_values so the
helper can be exercised without an NVLink-capable device, and asserts both
that it returns True and that it queried FieldId.DEV_NVLINK_GET_STATE. It
fails with AttributeError before this change.
@copy-pr-bot

copy-pr-bot Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions Bot added the cuda.bindings Everything related to the cuda.bindings module label Aug 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuda.bindings Everything related to the cuda.bindings module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant