Fix/157 #161
Fix/157 #161
Conversation
| all_paths = list(nx.all_simple_edge_paths(graph, source, target=target)) | ||
| max_path_length = 0 | ||
| idx = 0 | ||
| for jdx, path in enumerate(all_paths): | ||
| if len(path) > max_path_length: | ||
| max_path_length = len(path) | ||
| idx = jdx | ||
| return all_paths[idx] |
There was a problem hiding this comment.
| all_paths = list(nx.all_simple_edge_paths(graph, source, target=target)) | |
| max_path_length = 0 | |
| idx = 0 | |
| for jdx, path in enumerate(all_paths): | |
| if len(path) > max_path_length: | |
| max_path_length = len(path) | |
| idx = jdx | |
| return all_paths[idx] | |
| all_paths = list(nx.all_simple_edge_paths(graph, source, target=target)) | |
| return max(all_paths, key=len) |
Unless you want to deal with multiple longest paths
There was a problem hiding this comment.
I want to deal with it in the sense that there can be more than one with equivalent path length. But the smaller code should account for that I guess?
There was a problem hiding this comment.
Yes. Then you should get the one with the lowest index, but don't pin me on that.
| def polyply_custom_tree(graph, source, target): | ||
| # find all simple paths from source to target | ||
| longest_path = _find_longest_path(graph, source, target) | ||
| path_nodes = { u for edge in longest_path for u in edge } | ||
| # substract path and extract the remaining graphs of the branches | ||
| graph_copy = graph.copy() | ||
| graph_copy.remove_edges_from(longest_path) | ||
| components = list(nx.connected_components(graph_copy)) | ||
| # loop over the edges in the longest path and | ||
| # see if they are part of a connected component | ||
| # then add the bfs tree edges of that component to the | ||
| # graph before the edge | ||
| tree_graph = nx.DiGraph() | ||
| for from_node, to_node in longest_path: | ||
| for c in components: | ||
| if from_node in c: | ||
| break | ||
|
|
||
| subgraph = graph_copy.subgraph(c) | ||
| for u, v in nx.bfs_tree(subgraph, source=from_node).edges: | ||
| if u not in path_nodes or v not in path_nodes: | ||
| tree_graph.add_edge(u, v) | ||
| tree_graph.add_edge(from_node, to_node) | ||
|
|
||
| return tree_graph |
There was a problem hiding this comment.
Don't hate me, but did you see https://networkx.org/documentation/stable/reference/algorithms/tree.html#module-networkx.algorithms.tree.branchings and https://networkx.org/documentation/stable/reference/algorithms/tree.html#module-networkx.algorithms.tree.mst?
The edges that are in your graph but not in the tree are part of cycles that were broken to make the tree.
There was a problem hiding this comment.
yes but that is intended. The polyply search tree should be a directed graph without cycles. The tree should have been a separate PR but since it made it in. Basically the idea is to get rid of the dfs vs bfs thing for cycles. To do that I need to make a tree which essentially is a DFS tree along the backbone (i.e. longest path) but the branches are placed bfs like with cycles being replaced by distance restraints.
| target_node=None): | ||
|
|
||
| # graph distances can be computed from the path positions | ||
| graph_distances_ref = {node: path.index(node) for node in path} |
There was a problem hiding this comment.
Expensive! path.index costs N for the N=len(path)
There was a problem hiding this comment.
but its cheaper than computing the disjstra path length no?
There was a problem hiding this comment.
Sure. Consider however, the following:
{node: idx for (idx, node) in enumerate(path)}
First Draft of generalized restraint algorithm