[saiyambharara]

Description

Follow-up to #3170. Host callbacks run one at a time, so the event-wait case passed even with the wrong wait target. Now a sleep kernel delays the expected record, and an event the exec records after its wait must not land before it. If the streams serialize (CUDA_DEVICE_MAX_CONNECTIONS=1), it still passes but can't catch a wrong target.

Checklist

• New or existing tests cover these changes.
• The documentation is up to date with these changes.

[copy-pr-bot]

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.

[saiyambharara]

@Andy-Jost here's the follow-up. To check it, I simulated four node-update bugs that leave the exec waiting on the wrong event. The old case missed all four and the new one catches every one. 2000 repeats and 4-thread runs pass locally. Could you /ok to test 2803d30?

[coderabbitai]

📝 Summary

Summary by CodeRabbit

• Tests
  • Updated graph-node event-wait tests to cover synchronization between streams with timing-enabled events and a delayed producer. The checks now verify that the consumer proceeds only after the expected event timing is reached.

The event-wait test replaces callback-based synchronization with a nanosleep-delayed producer event. It launches the graph on a consumer stream, synchronizes both streams, and checks timestamp ordering.

Changes: Graph event-wait test

Layer / File(s)	Summary
Event-wait test synchronization cuda_core/tests/graph/test_graph_node_update.py	The test uses timing-enabled events and NanosleepKernel. It records the expected event after the delay, launches the graph on a consumer stream, and checks that the post-wait timestamp is no earlier than the expected timestamp.

Possibly related PRs

• NVIDIA/cuda-python#3170: This PR also updates the event-record graph test to use timing-enabled events.

Suggested labels: cuda.core

Suggested reviewers: andy-jost

Priority: Low

Change: Other

Merge Risk: Minimal · up to 2803d

This change only strengthens a graph event-wait test and does not alter library behavior. The remaining notes about older GPUs and serialized streams are low-impact and do not block merging.

Finishing Touches: 💡 1

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🔇 Additional comments: 2

cuda_core/tests/graph/test_graph_node_update.py-153-153: 1

153-153: Major · Functional Correctness · Quick win

⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

important: Support devices below compute capability 7.0.

NanosleepKernel compiles __nanosleep for the current device, but that instruction requires compute capability 7.0. CUDA 12 supports Maxwell and Pascal devices, so every event-wait case now fails during setup on those devices, including cases that do not launch the graph. Use a delay mechanism supported by those devices. (docs.nvidia.com)

cuda_core/tests/graph/test_graph_node_update.py-163-165: 1

163-165: Minor · Functional Correctness · Quick win

⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

important: Detect when the timing check cannot distinguish targets.

With CUDA_LAUNCH_BLOCKING=1, the delay finishes before graph.launch, so the assertion passes even if the graph waits on the wrong event. Stream serialization can produce the same result. Add a negative control and mark the check inconclusive when it cannot establish overlap. (docs.nvidia.com)

---

Review info

Run configuration

• Source mode: Auto
• Repository: NVIDIA/cuda-python
• Branch: main
• File: .coderabbit.yaml
• Commit: 94625c1
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Enterprise
• Run ID: 1b2af3da-5474-44a9-af46-993f1f62f5df

Commits

Reviewing files that changed from the base of the PR and between 94625c1 and 2803d30.

Files selected for processing: 1

• cuda_core/tests/graph/test_graph_node_update.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

[Andy-Jost]

/ok to test 2803d30

[Andy-Jost]

The rewrite does what the title says: the wait target is proven by device timestamps. after_wait - expected goes negative when the exec waits on the wrong event or on nothing, because only the correct wait sits behind the 20 ms kernel, and nothing in the path blocks a stream or queries CUDA while something blocks. One change before merge:

• _event_wait_case builds NanosleepKernel(device) with no compute-capability check. __nanosleep needs sm_70, and the other users of this helper skip below it (test_event.py, test_device.py, test_helpers.py, memory_ipc/test_event_ipc.py). On a Maxwell or Pascal device the five event-wait parametrizations now error in the factory instead of skipping. Add the same skip at the top of the factory, or build the kernel lazily in assert_exec_uses and skip there, so the two tests that never launch keep running on such devices:

  if device.compute_capability.major < 7:
      pytest.skip("__nanosleep is only available starting Volta (sm70)")

Not a blocker: the branch is based on main rather than on #3170, so this head still carries the old event-record case. The two merge cleanly in either order.

[github-actions]

Doc Preview CI
🚀 View preview at https://nvidia.github.io/cuda-python/pr-preview/pr-3262/
https://nvidia.github.io/cuda-python/pr-preview/pr-3262/cuda-core/
https://nvidia.github.io/cuda-python/pr-preview/pr-3262/cuda-bindings/
https://nvidia.github.io/cuda-python/pr-preview/pr-3262/cuda-pathfinder/
Preview will be ready when the GitHub Pages deployment is complete.