Fix operator precedence in test_cudart.supportsCudaAPI - #2554
Open
LeSingh1 wants to merge 1 commit into
Open
Conversation
def supportsCudaAPI(name):
return name in dir(cuda) or dir(cudart)
parses as `(name in dir(cuda)) or dir(cudart)`. `dir(cudart)` is a non-empty
list for any module, so it is unconditionally truthy and the function returns
a truthy value for every input, including names that exist nowhere.
The left operand is dead too: `cuda` is cuda.bindings.driver and every name
passed in is a cudaXxx runtime symbol. cudaGraphGetId, cudaGreenCtxCreate,
cudaDeviceGetExecutionCtx and cudaGraphConditionalHandleCreate are all defined
in runtime.pyx and appear nowhere in driver.pyx, so `name in dir(cuda)` is
always False and the result is always the `dir(cudart)` list.
Consequence: `not supportsCudaAPI(...)` is always False, so the API-presence
half of all 17 skipif guards that use it (lines 1443-1954) never fires. On a
build whose bindings genuinely lack the API, the test runs and dies with
AttributeError instead of skipping; only the driver_version_less_than() half
of each guard does any work.
Adds test_supportsCudaAPI, pinning all three cases: a runtime-only name, a
driver-only name, and a name that exists in neither. The last two fail before
this change.
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.
This parses as
(name in dir(cuda)) or dir(cudart).dir(cudart)is a non-empty list forany module, so it is unconditionally truthy — the function returns a truthy value for
every input, including names that exist nowhere:
The left operand is dead as well.
cudaiscuda.bindings.driver, and every name passedin is a
cudaXxxruntime symbol:driver.pyxruntime.pyxcudaGraphGetIdcudaGreenCtxCreatecudaDeviceGetExecutionCtxcudaGraphConditionalHandleCreateSo
name in dir(cuda)is alwaysFalseand the result is always thedir(cudart)list.Consequence
not supportsCudaAPI(...)is alwaysFalse, so the API-presence half of all 17skipifguards that use it never fires — lines 1443, 1501, 1528, 1575, 1617, 1646, 1693,1712, 1727, 1745, 1765, 1793, 1862, 1885, 1902, 1933, 1954. Only the
driver_version_less_than(...)half of each guard does any work.Those guards exist because the driver version and the binding version are independent:
cuda-bindingsbuilt against an older toolkit will not exposecudaGraphGetIdeven on a13.1 driver. Today such a build runs the test and dies with
AttributeErrorinstead ofskipping.
Fix
name in dir(cuda) or name in dir(cudart)— complete the second comparison. I kept thedir(cuda)half rather than narrowing to the runtime module: it is what the author wrote,it is harmless, and narrowing it would be a second, unrelated decision.
Test
test_supportsCudaAPIpins all three cases — a runtime-only name (cudaMalloc), adriver-only name (
cuInit), and a name that exists in neither. The last two fail onmain:supportsCudaAPI("cuInit")returns alist, notTrue, andsupportsCudaAPI("this_is_not_a_cuda_api")returns that same truthy list instead ofFalse. It needs no GPU and no driver —dir()does not load anything.ruff format --checkis clean.ruff checkreports one pre-existingI001on this filethat is byte-identical on
upstream/main; I left it alone as out of scope.