Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename misleading topological_sort to bfs_nodes #1590

Merged
merged 2 commits into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dace/codegen/targets/cuda.py
Original file line number Diff line number Diff line change
Expand Up @@ -2096,7 +2096,7 @@ def get_next_scope_entries(self, dfg, scope_entry):

# Get all non-sequential scopes from the same level
all_scopes = [
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not exactly sure why this does not use scope_subgraph instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, I'll try to look into this later. But for now I'd like to wrap up this PR. If we should change this, I think it should be in a separate PR.

But thanks for pointing it out. Looks like there's a potential to improve the code there.

node for node in parent_scope.topological_sort(scope_entry)
node for node in parent_scope.bfs_nodes(scope_entry)
if isinstance(node, nodes.EntryNode) and node.map.schedule != dtypes.ScheduleType.Sequential
]

Expand Down
2 changes: 1 addition & 1 deletion dace/codegen/targets/fpga.py
Original file line number Diff line number Diff line change
Expand Up @@ -1848,7 +1848,7 @@ def get_next_scope_entries(self, sdfg, dfg, scope_entry):
parent_scope = dfg.scope_subgraph(parent_scope_entry)

# Get all scopes from the same level
all_scopes = [node for node in parent_scope.topological_sort() if isinstance(node, dace.sdfg.nodes.EntryNode)]
all_scopes = [node for node in parent_scope.bfs_nodes() if isinstance(node, dace.sdfg.nodes.EntryNode)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is this not .nodes()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, a bit of a guess, but maybe because of the next line:

return all_scopes[all_scopes.index(scope_entry) + 1:]

.nodes() might return a different order.


return all_scopes[all_scopes.index(scope_entry) + 1:]

Expand Down
4 changes: 2 additions & 2 deletions dace/codegen/targets/framecode.py
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ def dispatch_state(state: SDFGState) -> str:
cft = cflow.structured_control_flow_tree(sdfg, dispatch_state)
else:
# If disabled, generate entire graph as general control flow block
states_topological = list(sdfg.topological_sort(sdfg.start_state))
states_topological = list(sdfg.bfs_nodes(sdfg.start_state))
last = states_topological[-1]
cft = cflow.GeneralBlock(dispatch_state, None,
[cflow.SingleState(dispatch_state, s, s is last) for s in states_topological], [],
Expand Down Expand Up @@ -553,7 +553,7 @@ def determine_allocation_lifetime(self, top_sdfg: SDFG):
array_names = sdfg.arrays.keys(
) #set(k for k, v in sdfg.arrays.items() if v.lifetime == dtypes.AllocationLifetime.Scope)
# Iterate topologically to get state-order
for state in sdfg.topological_sort():
for state in sdfg.bfs_nodes():
for node in state.data_nodes():
if node.data not in array_names:
continue
Expand Down
20 changes: 10 additions & 10 deletions dace/sdfg/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import networkx as nx
from dace.dtypes import deduplicate
import dace.serialize
from typing import Any, Callable, Generic, Iterable, List, Sequence, TypeVar, Union
from typing import Any, Callable, Generic, Iterable, List, Optional, Sequence, TypeVar, Union


class NodeNotFoundError(Exception):
Expand Down Expand Up @@ -364,19 +364,19 @@ def sink_nodes(self) -> List[NodeT]:
"""Returns nodes with no outgoing edges."""
return [n for n in self.nodes() if self.out_degree(n) == 0]

def topological_sort(self, source: NodeT = None) -> Sequence[NodeT]:
"""Returns nodes in topological order iff the graph contains exactly
one node with no incoming edges."""
def bfs_nodes(self, source: Optional[NodeT] = None) -> Iterable[NodeT]:
"""Returns an iterable over nodes traversed in breadth-first search
order starting from ``source``."""
if source is not None:
sources = [source]
else:
sources = self.source_nodes()
if len(sources) == 0:
sources = [self.nodes()[0]]
#raise RuntimeError("No source nodes found")
if len(sources) > 1:
sources = [self.nodes()[0]]
#raise RuntimeError("Multiple source nodes found")
if len(sources) != 1:
source = next(iter(self.nodes()), None)
if source is None:
return [] # graph has no nodes
sources = [source]

seen = OrderedDict() # No OrderedSet in Python
queue = deque(sources)
while len(queue) > 0:
Expand Down
2 changes: 1 addition & 1 deletion dace/sdfg/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -2563,7 +2563,7 @@ def _used_symbols_internal(self,
used_before_assignment = set() if used_before_assignment is None else used_before_assignment

try:
ordered_blocks = self.topological_sort(self.start_block)
ordered_blocks = self.bfs_nodes(self.start_block)
except ValueError: # Failsafe (e.g., for invalid or empty SDFGs)
ordered_blocks = self.nodes()

Expand Down
Loading