[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 checks to validate synchronization across producer and consumer streams using timing-enabled events and a delayed producer. The checks compare event timestamps to confirm the consumer stream does not proceed before the expected event. The timing-based assertion is skipped on devices below the supported compute capability. This replaces the previous callback-based synchronization setup.

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 event 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. The assertion is skipped below compute capability 7.

Suggested reviewers: andy-jost

Priority: Normal

Change: Other

Merge Risk: Low · up to 684c6

The change is mergeable, but the event-wait tests perform repeated CUDA kernel compilation that should be avoided for faster and more efficient test runs.

- [ ] 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.

[saiyambharara]

@Andy-Jost added the sm70 skip in 684c606, with the kernel built in assert_exec_uses as you suggested. Could you /ok to test 684c606?

[coderabbitai]

Actionable comments posted: 1

---

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: a4847470-2588-4a3f-8d22-86dcc6e1c461

Commits

Reviewing files that changed from the base of the PR and between 2803d30 and 684c606.

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.

[coderabbitai]

Minor · Performance & Scalability · Quick win

important: Avoid recompiling NanosleepKernel for every assertion.

The guard now runs inside assert_exec_uses, so NanosleepKernel(device) compiles once per execution check instead of once per case. This helper is called multiple times for the event-wait case. Move the capability check before case setup, then construct and reuse delay once. This also skips unsupported devices before creating case resources. (github.com)