docs(core): fix three cuda.core.system docs defects - #2584
Open
LeSingh1 wants to merge 1 commit into
Open
Conversation
1. Four Sphinx version directives are misspelled and render as errors.
cuda_core/cuda/core/system/_device.pyx:895 .. version-changed:: 1.1.0
cuda_core/cuda/core/system/_device.pyx:909 .. version-added:: 1.1.0
cuda_core/cuda/core/system/_device.pyx:919 .. version-added:: 1.1.0
cuda_core/cuda/core/system/_nvlink.pxi:31 .. version-deprecated:: 1.1.0
The directives are `versionchanged`, `versionadded` and `deprecated` --
no hyphen. The repository already spells `.. versionadded::` correctly in
31 other places (texture/, graph/), so these four are typos, not a local
convention. Sphinx emits "Unknown directive type" and drops the content,
which is how the behaviour change in `Device.get_nvlink` ("Any link number
not supported by this specific device will raise a ValueError") and the
deprecation of `NvlinkInfo.max_links` have been missing from the rendered
docs. The generated `_device.pyi` carries the same four typos and is fixed
with them.
2. `Device.get_cpu_affinity`'s docstring describes memory affinity.
Summary, fallback sentence and Returns section were copied from
`get_memory_affinity` twenty lines above, so they promise "indices of NUMA
nodes or CPU sockets" and "ideal memory affinity". The body calls
`nvmlDeviceGetCpuAffinityWithinScope`, sizes the bitmask in CPU words
(`ceil(cpu_count() / 64)`) and runs it through `_unpack_bitmask`, so the
list holds logical CPU indices. The docstring even contradicts itself: the
summary says "CPU affinity", the Returns section says "memory affinity".
A caller who believes it will index NUMA nodes with CPU numbers.
3. The `register_events` example calls `.event_type` on the wrong object.
`RegisteredSystemEvents.wait()` returns `SystemEvents`, which defines only
`__init__`, `__len__` and `__getitem__`; `event_type` lives on the
singular `SystemEvent`. So the documented snippet raises AttributeError.
The example was copied from the device-level one in _device.pyx:728-734,
where `DeviceEvents.wait()` really does return a single event. Rewritten to
index the batch, and to import `SystemEventType`, which the snippet used
without ever importing.
Contributor
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Three documentation defects in
cuda.core.system, all of which mislead or silently drop information in the rendered docs. Documentation only — no behaviour changes.1. Four Sphinx version directives are misspelled, so their content never renders
The directives are
versionchanged,versionaddedanddeprecated— no hyphen. Sphinx emitsUnknown directive typeand drops the body, so two notices users need are missing from the published docs:Device.get_nvlink— "Any link number not supported by this specific device will raise aValueError."NvlinkInfo.max_links— the deprecation notice pointing atDevice.get_nvlink_count.This is a typo rather than a local convention:
grepovercuda_core/,cuda_bindings/andcuda_python/finds.. versionadded::spelled correctly 31 times (intexture/,graph/, …) against 4 hyphenated ones, and 0 correct uses ofversionchanged/deprecatedanywhere — i.e. every hyphenated instance is in these two files.The generated
_device.pyistub carries the same four typos verbatim (stubgen-pyx copies the docstrings), so it is fixed alongside.2.
Device.get_cpu_affinity's docstring describes memory affinity, in the wrong unitsThe summary, the fallback sentence and the whole Returns section were copied from
get_memory_affinitytwenty lines above, so they promise "indices of NUMA nodes or CPU sockets" with the "ideal memory affinity". The body callsnvmlDeviceGetCpuAffinityWithinScope, sizes the bitmask in CPU words (ceil(cpu_count() / 64),:557) and runs it through_unpack_bitmask(_device_utils.pxi:8-24), so the returnedlist[int]holds logical CPU indices.The docstring even contradicts itself: the summary says "ideal CPU affinity", the Returns section says "ideal memory affinity". A caller who believes the Returns section will index NUMA nodes with CPU numbers. Corrected, with an explicit pointer to
get_memory_affinityfor the node/socket form.3. The
register_eventsexample calls.event_typeon the wrong objectRegisteredSystemEvents.wait()returnsSystemEvents(_system_events.pyx:143), which defines only__init__,__len__and__getitem__(:55-69);event_typelives on the singularSystemEvent(:34). The documented snippet therefore raisesAttributeError. It was copied from the device-level example at_device.pyx:728-734, whereDeviceEvents.wait()genuinely does return a single event.Rewritten to index the batch via the documented
__len__/__getitem__(rather than relying on the legacy sequence-iteration protocol), and to importSystemEventType, which the snippet used without ever importing.What I ran
Environment: macOS, no CUDA driver and no CUDA toolkit, so
cuda.corecannot be built or imported here, and Sphinx is not installed.main:.. versionadded::31 uses vs.. version-added::4;.. versionchanged::0 vs.. version-changed::2;.. deprecated::0 vs.. version-deprecated::2. All 8 hyphenated uses are the ones changed here (4 in.pyx/.pxi, 4 in the generated.pyi); no hyphenated directive remains anywhere in the tree afterwards.SystemEventsreally has noevent_type(_system_events.pyx:55-69), andSystemEventTypeis exported fromcuda.core.system.typing(typing.py:21,247) — which is the import path_system_events.pyx:13itself uses.get_cpu_affinitycallsnvml.device_get_cpu_affinity_within_scope, not the memory variant, and both helpers feed_unpack_bitmask, which yields set-bit indices.get_memory_affinitysizes its NUMA-node bitmask withceil(cpu_count() / 64)— a CPU count used for a node-set length. It over-allocates, which NVML tolerates, so I could not prove a wrong result and left it alone.