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

@LeSingh1 LeSingh1 commented Aug 9, 2026

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:

    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)

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)

    for idx in range(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 one CUmemAccessDesc, 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-pr-bot Bot commented Aug 9, 2026

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

LeSingh1 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

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 free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuda.bindings Everything related to the cuda.bindings module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant