Skip to content

Commit

Permalink
Fixing return ordering of depth_first_traversal_by
Browse files Browse the repository at this point in the history
Summary:
The original is D57905416.
`preorder-left-to-right` by default was backed out D58097382 due to runtime failures. Investigation showed that it was not implemented correctly.
`depth_first_traversal_by` does two things:
1. Calls `get_nodes_to_traverse_func` for each node. That was called with correct `preorder-left-to-right` order as expected.
2. Returns the list of all elements that was walked throught. **We had an error here**. We can't use `visited` list for returning results. `visited` does *not* always ordered in the same way. If we have more than one `roots` passed the order was broken.

Solving it by defining explicit list for return and fill it in the exact same way I do traversal. Now everything works!

Reviewed By: artempyanykh

Differential Revision: D58137945

fbshipit-source-id: 53aacdd3b833de48e7e91e01a2ef10fefee81252
  • Loading branch information
Nikita Patskov authored and facebook-github-bot committed Jun 4, 2024
1 parent f788f51 commit 1f8c948
Showing 1 changed file with 5 additions and 2 deletions.
7 changes: 5 additions & 2 deletions utils/graph_utils.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ def depth_first_traversal_by(
graph_nodes: [dict[typing.Any, typing.Any], None],
roots: list[typing.Any],
get_nodes_to_traverse_func: typing.Callable,
traversal: GraphTraversal = GraphTraversal("preorder-right-to-left"),
traversal: GraphTraversal = GraphTraversal("preorder-left-to-right"),
node_formatter: typing.Callable[[typing.Any], str] = str) -> list[typing.Any]:
"""
Performs a depth first traversal of `graph_nodes`, beginning
Expand All @@ -216,6 +216,7 @@ def depth_first_traversal_by(

# Dictify for O(1) lookup
visited = {k: None for k in roots}
result = []
stride = -1 if traversal == GraphTraversal("preorder-left-to-right") else 1

stack = []
Expand All @@ -226,6 +227,8 @@ def depth_first_traversal_by(
if not stack:
break
node = stack.pop()
result.append(node)

if graph_nodes and node not in graph_nodes:
fail("Expected node {} in graph nodes".format(node_formatter(node)))
nodes_to_visit = get_nodes_to_traverse_func(node)
Expand All @@ -237,4 +240,4 @@ def depth_first_traversal_by(

expect(not stack, "Expected to be done with graph traversal stack.")

return visited.keys()
return result

0 comments on commit 1f8c948

Please sign in to comment.