Skip to content

cuda.core: add GraphBuilder.graph_definition property - #2026

Merged
Andy-Jost merged 3 commits into
NVIDIA:mainfrom
Andy-Jost:graph-step-3
Jul 6, 2026
Merged

cuda.core: add GraphBuilder.graph_definition property#2026
Andy-Jost merged 3 commits into
NVIDIA:mainfrom
Andy-Jost:graph-step-3

Conversation

@Andy-Jost

Copy link
Copy Markdown
Contributor

Summary

Completes step 3 of #1330 by exposing the captured graph as an explicit GraphDefinition view that shares ownership of the same graph the builder is producing. The handle-layer plumbing landed in #2008; this PR wires up the user-facing surface and the state-guard rules.

The new property unlocks two hybrid flows:

  • Inspect or augment a captured graph through the explicit API after end_building(), then re-complete() to pick up the changes.
  • Populate a conditional body (returned by if_then / if_else / while_loop / switch) entirely through the explicit API without ever calling begin_building() on it.

API addition

GraphBuilder.graph_definition: GraphDefinition (read-only property)

Availability rules:

Builder kindBefore begin_buildingWhile buildingAfter end_building
PrimaryRuntimeErrorRuntimeErrorOK
Conditional bodyOKRuntimeErrorOK
ForkedRuntimeErrorRuntimeErrorRuntimeError

The returned GraphDefinition is a view, not an owning wrapper: nodes added through it appear in subsequent complete() and debug_dot_print() calls on the builder.

Test plan

Nine new tests in test_graph_builder.py:

  • Happy path: graph_definition returns a GraphDefinition after end_building() and reflects the captured nodes.
  • Three error paths: forked builder, primary pre-begin_building, any builder mid-capture.
  • Shared validity: the returned GraphDefinition keeps working after the builder is closed.
  • Round-trip: mutate via the explicit API after capture, complete(), and run end-to-end on a stream.
  • Conditional-body hybrid flow: populate the body entirely through the explicit API and run.
  • Conditional-body augmentation flow: capture into the body, then add extra nodes through the explicit view, and run.
  • Conditional-body during-capture rejection.

Local pre-commit clean. Local test run on a 2-GPU machine: all cuda_core/tests/graph pass.

Related

Made with Cursor

@Andy-JostAndy-Jost added this to the cuda.core v1.0.0 milestone May 5, 2026
@Andy-JostAndy-Jost added enhancement Any code-related improvements P1 Medium priority - Should do cuda.core Everything related to the cuda.core module labels May 5, 2026
@Andy-JostAndy-Jost self-assigned this May 5, 2026
@Andy-Jost

Copy link
Copy Markdown
ContributorAuthor

The builds on #2008. Commits unique to this PR begin at de85b92adfc0ea6c975a2259cfc2c6ef185ca430

@github-actions

This comment has been minimized.

Completes step 3 of NVIDIA#1330 by exposing the captured graph as an explicit
`GraphDefinition` view that shares ownership of the underlying `CUgraph`.
The handle-layer plumbing landed in PR NVIDIA#2008; this commit wires up the
user-facing surface and locks in the state-guard rules.
State semantics:
- PRIMARY builder: only valid after `end_building()`. Before
`begin_building()` no graph exists; during capture the driver is the
sole writer, so explicit access is unsafe.
- CONDITIONAL_BODY builder: valid both before `begin_building()` (the
body graph is allocated at conditional-node creation time) and after
`end_building()`. This enables a hybrid flow where a conditional body
is populated entirely via the explicit API, with no capture at all.
- FORKED builder: never valid. Forked builders share the primary's
graph; access through the primary instead.
Tests cover the happy path, both hybrid flows on conditional bodies
(populate-via-explicit-API and capture-then-augment), the three error
states (forked, capturing, primary pre-capture), and the
shared-ownership guarantee (the `GraphDefinition` survives the
builder's `close()`).
Co-authored-by: Cursor <cursoragent@cursor.com>
@rwgk

rwgk commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

gpt-5.5 (prompt was, roughly: fully review of technical details, with moderate consideration of high-level aspects)

Findings

  • P2: GraphBuilder.graph_definition can be accessed after gb.close(), but close() has reset _h_graph, so the property returns a GraphDefinition wrapper around an empty graph handle. Calls on that wrapper then reach CUDA with a null CUgraph instead of failing at the builder boundary with the same clear closed-state behavior as complete() and debug_dot_print(). A GraphDefinition obtained before close() should keep working; accessing the property after close() should be rejected. See cuda_core/cuda/core/graph/_graph_builder.pyx:267 and cuda_core/cuda/core/graph/_graph_builder.pyx:326.
  • P3: The new public GraphBuilder.graph_definition API is missing the repo-required API lifecycle documentation. cuda_core/AGENTS.md says new public APIs must carry @versionadded or an explicit .. versionadded:: directive, and the 1.1.0 release notes have no entry for this user-facing graph API. Add the version annotation to the property docstring/stub and a release-note bullet so users can discover the feature. See cuda_core/cuda/core/graph/_graph_builder.pyx:286, cuda_core/cuda/core/graph/_graph_builder.pyi:133, and cuda_core/docs/source/release/1.1.0-notes.rst:10.
  • P3: The newly added tests do not include the repository’s explicit authorship markers. The root AGENTS.md requires one of agent_authored, human_reviewed, or human_authored immediately above each newly added unit test. The nine new test_graph_definition_* tests are currently unmarked, starting at cuda_core/tests/graph/test_graph_builder.py:395; the conditional tests at cuda_core/tests/graph/test_graph_builder.py:488, cuda_core/tests/graph/test_graph_builder.py:527, and cuda_core/tests/graph/test_graph_builder.py:563 also need a marker in addition to their existing requires_module decorator.

Address review feedback on PR NVIDIA#2026:
- Reject GraphBuilder.graph_definition access after close() via
GB_check_open, so it fails at the builder boundary instead of
returning a GraphDefinition wrapping a null CUgraph. A view obtained
before close() still works (shared ownership).
- Add ".. versionadded:: 1.1.0" to the property (.pyx and .pyi) and a
release-note bullet in 1.1.0-notes.rst.
- Add pytest authorship markers to the new graph_definition tests and a
test for the post-close rejection path.
@Andy-Jost

Copy link
Copy Markdown
ContributorAuthor

All comments should be addressed in the latest commit.

rwgk
rwgk approved these changes Jul 2, 2026

@rwgkrwgk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I manually reviewed the production code changes.

I trust that the tests are fine: generated by claude-opus-4.8, reviewed by codex gpt-5.5.

@Andy-Jost
Andy-Jost merged commit 5be658a into NVIDIA:mainJul 6, 2026
181 of 184 checks passed
@Andy-Jost
Andy-Jost deleted the graph-step-3 branch July 6, 2026 16:08
@github-actions

Copy link
Copy Markdown
Doc Preview CI
Preview removed because the pull request was closed or merged.

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

Labels

cuda.coreEverything related to the cuda.core moduleenhancementAny code-related improvementsP1Medium priority - Should do

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Andy-Jost@rwgk@leofang