Skip to content

fix(bindings): two silent wrong results in the vector_add examples - #2575

Open
LeSingh1 wants to merge 1 commit into
NVIDIA:mainfrom
LeSingh1:examples-vector-add-silent-pass
Open

fix(bindings): two silent wrong results in the vector_add examples#2575
LeSingh1 wants to merge 1 commit into
NVIDIA:mainfrom
LeSingh1:examples-vector-add-silent-pass

Conversation

@LeSingh1

Copy link
Copy Markdown
Contributor

Two independent defects in the vector_add examples, both of which produce a wrong result rather than an error. Grouped because they are the same pair of files and the same failure mode.

1. A wrong value in the last element is reported as a pass

vector_add_drv.py:104-117 and vector_add_mmap.py:294-307:

foriinrange(n):
sum_all=h_a[i] +h_b[i]
ifmath.fabs(h_c[i] -sum_all) >1e-7:
break
...
ifi+1!=n:
print("Result = FAIL", file=sys.stderr)
sys.exit(1)

This is the C sample's if (i == N) PASS, which works only because a completed C for loop leaves i == N. A completed Python loop leaves i == n - 1, so the check was rewritten as i + 1 != n — but break at the final index n - 1 yields i + 1 == n as well. A kernel that computes h_c[n-1] incorrectly therefore exits 0 and prints nothing.

Fixed with an explicit flag, which does not depend on where the loop stopped.

2. simple_malloc_multi_device_mmap grants access to only the last device

vector_add_mmap.py:161:

access_descriptors= [cuda.CUmemAccessDesc()] *len(mapping_devices)
foridxinrange(len(mapping_devices)):
access_descriptors[idx].location.type= ...
access_descriptors[idx].location.id=mapping_devices[idx]
access_descriptors[idx].flags= ...
(status,) =cuda.cuMemSetAccess(dptr, size, access_descriptors, len(access_descriptors))

List multiplication stores N references to oneCUmemAccessDesc, which is mutable, so the loop overwrites the same object N times and cuMemSetAccess receives mapping_devices[-1] repeated N times. The call succeeds; the devices that were supposed to be granted access instead fault on first touch.

The helper is written as a general multi-device routine — that is what mapping_devices and the surrounding comments are for — and the bug is masked today only because main() passes a single device (vector_add_mmap.py:235). Fixed with a list comprehension.

What I ran

Environment: macOS, no CUDA driver and no CUDA toolkit.

  • Did not run: either example, or cuda_bindings/tests/test_examples.py — both need a GPU, and vector_add_mmap additionally needs VMM support. Note that test_examples.py would not catch either defect anyway: it runs each example with no arguments on a machine where the kernel is correct, so neither the last-element path nor a multi-device mapping_devices is ever exercised.
  • Ran: reductions of both idioms, verbatim, before and after:
--- verification loop ---
all correct before -> PASS after -> PASS
h_c[0] wrong before -> FAIL after -> FAIL
h_c[n-1] wrong before -> PASS after -> FAIL <-- the defect
--- access descriptors ---
before -> [desc(device_id=2), desc(device_id=2), desc(device_id=2)]
after -> [desc(device_id=0), desc(device_id=1), desc(device_id=2)]
  • Ran:python -m py_compile, ruff check, ruff format --check on both files — clean, no new findings against a main baseline for the same files.
  • Checked:i is not read anywhere else in either function after the loop, and [...] * len(...) on a mutable element appears nowhere else in cuda_bindings/examples or cuda_core/examples.

1. A wrong value in the LAST element is reported as a pass.
Both examples transliterate the C sample's success test:
for i in range(n):
sum_all = h_a[i] + h_b[i]
if math.fabs(h_c[i] - sum_all) > 1e-7:
break
...
if i + 1 != n:
print("Result = FAIL", file=sys.stderr)
sys.exit(1)
In C, `if (i == N)` works because a completed loop leaves `i == N`. In
Python a completed loop leaves `i == n - 1`, hence the `i + 1 != n`
rewrite -- but `break` at the final index `n - 1` produces `i + 1 == n`
too. So a kernel that computes h_c[n-1] wrongly exits 0 and prints nothing.
Use an explicit flag, which does not depend on where the loop stopped.
2. simple_malloc_multi_device_mmap grants access to only the last device.
access_descriptors = [cuda.CUmemAccessDesc()] * len(mapping_devices)
List multiplication stores N references to ONE mutable CUmemAccessDesc, so
the loop that fills in location.id overwrites the same object N times and
cuMemSetAccess receives mapping_devices[-1] repeated N times. It succeeds;
the devices that were supposed to be granted access instead fault on first
touch. The helper is written as a general multi-device routine -- that is
what its docstring and its mapping_devices parameter are for -- and is
masked today only because main() passes a single device.
Neither is reachable from CI: tests/test_examples.py runs each example with no
arguments on a machine where the kernel is correct.
@copy-pr-bot

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-actionsgithub-actionsBot added the cuda.bindings Everything related to the cuda.bindings module label Aug 9, 2026
@LeSingh1

Copy link
Copy Markdown
ContributorAuthor

Heads-up on overlap: #2266 (samples migration) deletes both of these files. If that lands first, these two fixes should travel with the migrated copies instead — happy to close this and re-target, or to leave it as the record of what needs carrying over. Flagging rather than guessing.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuda.bindingsEverything related to the cuda.bindings module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@LeSingh1