fix(bindings): repair the two example command-line paths that #2546 revives - #2579
Open
LeSingh1 wants to merge 1 commit into
Open
fix(bindings): repair the two example command-line paths that #2546 revives#2579LeSingh1 wants to merge 1 commit into
LeSingh1 wants to merge 1 commit into
Conversation
…2546 revives Both defects sit behind an argv option, and `check_cmd_line_flag` currently always returns False (enumerate unpacked backwards -- see NVIDIA#2546), so neither can fire today. Fixing the helper turns both into immediate failures, and tests/test_examples.py runs every example with no arguments, so CI will not catch either. 1. simple_zero_copy.py: cudaGetDeviceCount() is not unwrapped. device_count = cudart.cudaGetDeviceCount() idev = int(get_cmd_line_argument_int("device=")) if idev >= device_count or idev < 0: The bindings return (cudaError_t, count), so the comparison raises "TypeError: '>=' not supported between instances of 'int' and 'tuple'". Every other call site in the examples wraps it, e.g. simple_p2p.py:52. 2. global_to_shmem_async_copy.py: grid_shared_state_kernel.z is never set. C++ `dim3` defaults every component to 1, which is why the C sample can write `dim3 gridSharedStateKernel(a, b)`. `cudart.dim3` is a cdef class over a zero-initialised struct, so .z stays 0 -- and it is passed as gridDimZ at both cuLaunchKernel sites for AsyncCopyMultiStageSharedState (kernel=3), which the driver rejects with CUDA_ERROR_INVALID_VALUE. Every other dim3 in the file and in the sibling examples sets .z explicitly, including the `grid` twelve lines above. Refs NVIDIA#2546
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.
Two defects that both sit behind an argv option.
check_cmd_line_flagcurrently always returnsFalse(enumerateunpacked backwards — that is #2546), so neither can fire today; fixing the helper turns both into immediate failures.tests/test_examples.pyruns every example with no arguments, so CI catches neither. Grouped because they are the same class and the same trigger.1.
simple_zero_copy.py:cudaGetDeviceCount()is not unwrappedcuda_bindings/examples/0_Introduction/simple_zero_copy.py:71-75:The bindings return
(cudaError_t, count)(runtime.pyx:21309), sodevice_countis a tuple and the comparison raises:Every other call site in the examples wraps it — e.g.
simple_p2p.py:52,gpu_n = check_cuda_errors(cudart.cudaGetDeviceCount()). Fixed the same way.2.
global_to_shmem_async_copy.py:grid_shared_state_kernel.zis never setcuda_bindings/examples/3_CUDA_Features/global_to_shmem_async_copy.py:795-801:C++
dim3defaults every component to 1, which is why the C sample can writedim3 gridSharedStateKernel(a, b).cudart.dim3is acdef classwhose backing struct is zero-initialised (runtime.pyx:7639-7643—self._pvt_ptr = &self._pvt_val), so.zstays 0. It is then passed asgridDimZat bothcuLaunchKernelsites forKernels.AsyncCopyMultiStageSharedState(lines 866 and 1003), which the driver rejects withCUDA_ERROR_INVALID_VALUE.Every other
dim3in the file and in the sibling examples sets.zexplicitly — includinggridtwelve lines above (grid.z = 1),threads_shared_state_kernel.zimmediately before it,simple_cubemap_texture.py:162, andsimple_p2p.py:151— so this one omission is clearly unintended. Reachable viakernel=3.Deliberately not touched:
grid_shared_state_kernel.y = dims_a.y / threads_shared_state_kernel.xuses.xwhere the siblinggrid.yuses.y. That matches the upstream C sample and is intentional — the 16x18 block has only 16 consumer rows.What I ran
Environment: macOS, no CUDA driver and no CUDA toolkit.
cuda_bindings/tests/test_examples.py— both need a GPU, and Docs #2 additionally needskernel=3, which is unreachable until Fix command line parsing in the cuda.bindings example helpers #2546 lands.python -m py_compile,ruff check,ruff format --checkon both files — clean, no new findings against amainbaseline.cudart.dim3().z == 0. I could not importcuda.bindingshere, so this rests onruntime.pyx:7639-7643(the wrapper points at an in-object struct member, which CPython zero-fills on allocation) plus the fact thatdim3exposes no constructor arguments other than_ptr.Overlap notes:
simple_zero_copy.pyis one of the files #2266 (samples migration) deletes;global_to_shmem_async_copy.pyis not. If #2266 lands first, fix #1 should travel with the migrated copy — flagging rather than guessing.Refs #2546