Report host_numa virtual memory as host-accessible - #2503
Open
LeSingh1 wants to merge 1 commit into
Open
Conversation
`VirtualMemoryResource.__init__` classifies "host", "host_numa" and "host_numa_current" all as host-located (it clears `self.device` for each), but `is_host_accessible` compared with `== "host"`. A resource configured with `location_type="host_numa"` or `"host_numa_current"` therefore reported `is_host_accessible is False` *and* `is_device_accessible is False` -- an impossible answer that propagates to `Buffer.is_host_accessible`, which forwards to the memory resource. Share a single `_HOST_LOCATION_TYPES` set between the constructor and the property so the two classifications cannot drift again.
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.
A
VirtualMemoryResourceconfigured withlocation_type="host_numa"or"host_numa_current"reportsis_host_accessible is Falseandis_device_accessible is False— it claims the memory is reachable from nowhere.__init__classifies host-located resources with a substring test:but
is_host_accessibleused exact equality:Both were introduced in cec0efb (#2016) and disagreed from the start. The answer propagates to
Buffer.is_host_accessible, which forwards straight to the memory resource, so anything choosing CPU vs GPU from a buffer — DLPack/StridedMemoryViewdevice selection, copy validation — sees the wrong answer.host_numa_currentis a working configuration today: the driver ignoreslocation.idfor that type, so the resource allocates fine and only the reported accessibility is wrong.The fix gives both sites a shared
_HOST_LOCATION_TYPESfrozenset so they cannot drift again.VirtualMemoryLocationTypeis aStrEnum, so membership behaves identically for raw strings and enum members.is_device_accessibleis deliberately left alone.On verification, plainly: I have no GPU and no
cuda.bindingsbuild here, so nothing undercuda_core/tests/can even import and I did not run the new test. What I did run is the old and new expressions plus__init__'s classifier extracted against a verbatim copy of the enum — that reproduces both NUMA types returning False on both properties before the change, and agreement with__init__for all four members after. The regression test is GPU-gated by construction and will fail onmainfor the two NUMA parameters.NOTE: developed with the assistance of an AI coding agent. The new test carries
@pytest.mark.agent_authoredper AGENTS.md. I reviewed and verified the change before submitting.