Skip to content

Commit 9a2d5f4

Browse files
committed
Resolve some review comments
1 parent de22d5f commit 9a2d5f4

2 files changed

Lines changed: 33 additions & 32 deletions

File tree

cuda_core/cuda/core/experimental/_graph.py

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import weakref
88
from dataclasses import dataclass
9-
from typing import TYPE_CHECKING, Optional, Tuple
9+
from typing import TYPE_CHECKING, Optional
1010

1111
if TYPE_CHECKING:
1212
from cuda.core.experimental._stream import Stream
@@ -165,7 +165,7 @@ def close(self):
165165
def __init__(self):
166166
raise NotImplementedError(
167167
"directly creating a Graph object can be ambiguous. Please either "
168-
"call Device.create_graph_builder() or stream.creating_graph_builder()"
168+
"call Device.create_graph_builder() or stream.create_graph_builder()"
169169
)
170170

171171
@classmethod
@@ -374,7 +374,7 @@ def debug_dot_print(self, path, options: Optional[DebugPrintOptions] = None):
374374

375375
handle_return(driver.cuGraphDebugDotPrint(self._mnff.graph, path, flags))
376376

377-
def split(self, count) -> Tuple[GraphBuilder, ...]:
377+
def split(self, count: int) -> tuple[GraphBuilder, ...]:
378378
"""Splits the original graph builder into multiple graph builders.
379379
380380
The new builders inherit work dependencies from the original builder.
@@ -387,7 +387,7 @@ def split(self, count) -> Tuple[GraphBuilder, ...]:
387387
388388
Returns
389389
-------
390-
graph_builders : Tuple[:obj:`~_graph.GraphBuilder`, ...]
390+
graph_builders : tuple[:obj:`~_graph.GraphBuilder`, ...]
391391
A tuple of split graph builders. The first graph builder in the tuple
392392
is always the original graph builder.
393393
@@ -423,7 +423,7 @@ def join(*graph_builders) -> GraphBuilder:
423423
The newly joined graph builder.
424424
425425
"""
426-
if not all(isinstance(builder, GraphBuilder) for builder in graph_builders):
426+
if any(not isinstance(builder, GraphBuilder) for builder in graph_builders):
427427
raise TypeError("All arguments must be GraphBuilder instances")
428428
if len(graph_builders) < 2:
429429
raise ValueError("Must join with at least two graph builders")
@@ -436,15 +436,16 @@ def join(*graph_builders) -> GraphBuilder:
436436
break
437437

438438
# Join all onto the root builder
439+
root_bdr = graph_builders[root_idx]
439440
for idx, builder in enumerate(graph_builders):
440441
if idx == root_idx:
441442
continue
442-
graph_builders[root_idx].stream.wait(builder.stream)
443+
root_bdr.stream.wait(builder.stream)
443444
builder.close()
444445

445-
return graph_builders[root_idx]
446+
return root_bdr
446447

447-
def __cuda_stream__(self) -> Tuple[int, int]:
448+
def __cuda_stream__(self) -> tuple[int, int]:
448449
"""Return an instance of a __cuda_stream__ protocol."""
449450
return self.stream.__cuda_stream__()
450451

@@ -555,7 +556,7 @@ def if_cond(self, handle: int) -> GraphBuilder:
555556
node_params.conditional.ctx = self._get_conditional_context()
556557
return self._cond_with_params(node_params)[0]
557558

558-
def if_else(self, handle: int) -> Tuple[GraphBuilder, GraphBuilder]:
559+
def if_else(self, handle: int) -> tuple[GraphBuilder, GraphBuilder]:
559560
"""Adds an if-else condition branch and returns new graph builders for both branches.
560561
561562
The resulting if graph will execute the branch if the conditional handle
@@ -570,7 +571,7 @@ def if_else(self, handle: int) -> Tuple[GraphBuilder, GraphBuilder]:
570571
571572
Returns
572573
-------
573-
graph_builders : Tuple[:obj:`~_graph.GraphBuilder`, :obj:`~_graph.GraphBuilder`]
574+
graph_builders : tuple[:obj:`~_graph.GraphBuilder`, :obj:`~_graph.GraphBuilder`]
574575
A tuple of two new graph builders, one for the if branch and one for the else branch.
575576
576577
"""
@@ -586,7 +587,7 @@ def if_else(self, handle: int) -> Tuple[GraphBuilder, GraphBuilder]:
586587
node_params.conditional.ctx = self._get_conditional_context()
587588
return self._cond_with_params(node_params)
588589

589-
def switch(self, handle: int, count: int) -> Tuple[GraphBuilder, ...]:
590+
def switch(self, handle: int, count: int) -> tuple[GraphBuilder, ...]:
590591
"""Adds a switch condition branch and returns new graph builders for all cases.
591592
592593
The resulting switch graph will execute the branch that matches the
@@ -604,7 +605,7 @@ def switch(self, handle: int, count: int) -> Tuple[GraphBuilder, ...]:
604605
605606
Returns
606607
-------
607-
graph_builders : Tuple[:obj:`~_graph.GraphBuilder`, ...]
608+
graph_builders : tuple[:obj:`~_graph.GraphBuilder`, ...]
608609
A tuple of new graph builders, one for each branch.
609610
610611
"""

cuda_core/tests/test_graph.py

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ def test_graph_conditional_if(init_cuda, condition_value):
244244
launch(gb_if, LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data)
245245
gb_if_0, gb_if_1 = gb_if.split(2)
246246
launch(gb_if_0, LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data)
247-
launch(gb_if_1, LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data + 4)
247+
launch(gb_if_1, LaunchConfig(grid=1, block=1), add_one, arr[1:].ctypes.data)
248248
gb_if = GraphBuilder.join(gb_if_0, gb_if_1)
249249
launch(gb_if, LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data)
250250
gb_if.end_building()
@@ -309,16 +309,16 @@ def test_graph_conditional_if_else(init_cuda, condition_value):
309309
launch(gb_if, LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data)
310310
gb_if_0, gb_if_1 = gb_if.split(2)
311311
launch(gb_if_0, LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data)
312-
launch(gb_if_1, LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data + 4)
312+
launch(gb_if_1, LaunchConfig(grid=1, block=1), add_one, arr[1:].ctypes.data)
313313
gb_if = GraphBuilder.join(gb_if_0, gb_if_1)
314314
launch(gb_if, LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data)
315315
gb_if.end_building()
316316

317317
## ELSE nodes
318318
gb_else = gb_else.begin_building()
319-
launch(gb_else, LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data + 4)
320-
launch(gb_else, LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data + 4)
321-
launch(gb_else, LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data + 4)
319+
launch(gb_else, LaunchConfig(grid=1, block=1), add_one, arr[1:].ctypes.data)
320+
launch(gb_else, LaunchConfig(grid=1, block=1), add_one, arr[1:].ctypes.data)
321+
launch(gb_else, LaunchConfig(grid=1, block=1), add_one, arr[1:].ctypes.data)
322322
gb_else.end_building()
323323

324324
# Add Node C (...)
@@ -386,18 +386,18 @@ def test_graph_conditional_switch(init_cuda, condition_value):
386386

387387
## Case 1
388388
gb_case[1].begin_building()
389-
launch(gb_case[1], LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data + 4)
389+
launch(gb_case[1], LaunchConfig(grid=1, block=1), add_one, arr[1:].ctypes.data)
390390
gb_case_1_left, gb_case_1_right = gb_case[1].split(2)
391-
launch(gb_case_1_left, LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data + 4)
392-
launch(gb_case_1_right, LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data + 8)
391+
launch(gb_case_1_left, LaunchConfig(grid=1, block=1), add_one, arr[1:].ctypes.data)
392+
launch(gb_case_1_right, LaunchConfig(grid=1, block=1), add_one, arr[2:].ctypes.data)
393393
gb_case[1] = GraphBuilder.join(gb_case_1_left, gb_case_1_right)
394394
gb_case[1].end_building()
395395

396396
## Case 2
397397
gb_case[2].begin_building()
398-
launch(gb_case[2], LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data + 8)
399-
launch(gb_case[2], LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data + 8)
400-
launch(gb_case[2], LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data + 8)
398+
launch(gb_case[2], LaunchConfig(grid=1, block=1), add_one, arr[2:].ctypes.data)
399+
launch(gb_case[2], LaunchConfig(grid=1, block=1), add_one, arr[2:].ctypes.data)
400+
launch(gb_case[2], LaunchConfig(grid=1, block=1), add_one, arr[2:].ctypes.data)
401401
gb_case[2].end_building()
402402

403403
# Add Node C (...)
@@ -499,9 +499,9 @@ def test_graph_child_graph(init_cuda):
499499

500500
# Capture the child graph
501501
gb_child = Device().create_graph_builder().begin_building()
502-
launch(gb_child, LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data + 4)
503-
launch(gb_child, LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data + 4)
504-
launch(gb_child, LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data + 4)
502+
launch(gb_child, LaunchConfig(grid=1, block=1), add_one, arr[1:].ctypes.data)
503+
launch(gb_child, LaunchConfig(grid=1, block=1), add_one, arr[1:].ctypes.data)
504+
launch(gb_child, LaunchConfig(grid=1, block=1), add_one, arr[1:].ctypes.data)
505505
gb_child.end_building()
506506

507507
# Capture the parent graph
@@ -576,16 +576,16 @@ def build_graph(condition_value):
576576

577577
## Case 1
578578
gb_case[1].begin_building()
579-
launch(gb_case[1], LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data + 4)
580-
launch(gb_case[1], LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data + 4)
581-
launch(gb_case[1], LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data + 4)
579+
launch(gb_case[1], LaunchConfig(grid=1, block=1), add_one, arr[1:].ctypes.data)
580+
launch(gb_case[1], LaunchConfig(grid=1, block=1), add_one, arr[1:].ctypes.data)
581+
launch(gb_case[1], LaunchConfig(grid=1, block=1), add_one, arr[1:].ctypes.data)
582582
gb_case[1].end_building()
583583

584584
## Case 2
585585
gb_case[2].begin_building()
586-
launch(gb_case[2], LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data + 8)
587-
launch(gb_case[2], LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data + 8)
588-
launch(gb_case[2], LaunchConfig(grid=1, block=1), add_one, arr.ctypes.data + 8)
586+
launch(gb_case[2], LaunchConfig(grid=1, block=1), add_one, arr[2:].ctypes.data)
587+
launch(gb_case[2], LaunchConfig(grid=1, block=1), add_one, arr[2:].ctypes.data)
588+
launch(gb_case[2], LaunchConfig(grid=1, block=1), add_one, arr[2:].ctypes.data)
589589
gb_case[2].end_building()
590590

591591
return gb.end_building()

0 commit comments

Comments
 (0)