Found the bug

treespace/create_trees.py

@@ -26,6 +26,10 @@ def continue_building_tree(tree: DiGraph, g: DiGraph) -> bool:
             root_path = path
 
     # TODO: A leaf path might need to be manually connected to a root if it has such a parent! Bug in my approach!
+    # Print the adjacency list of the tree
+    print("[Checking Building Tree] Adjacency List of Tree")
+    for node, neighbors in tree.adj.items():
+        print(f"{node}: {list(neighbors)}")
 
     print("[Checking Building Tree] Root Path", root_path)
     print("[Checking Building Tree] Non-Root Paths", non_root_paths)
@@ -66,6 +70,7 @@ def combine_paths_based_on_edge(graph: DiGraph, updating_path: List, current_lea
     Returns:
         DiGraph: the updated tree with the new path added
     """
+    # TODO: BUG, you should explicitly have an LCA prepended to new_path!
     # Identify the nodes in the first path up to the start of the edge to add
     path1_nodes = updating_path[:updating_path.index(edge_to_add[0])+1]
 
@@ -74,9 +79,9 @@ def combine_paths_based_on_edge(graph: DiGraph, updating_path: List, current_lea
 
     # Combine these nodes to create the new path
     new_path = path1_nodes + path2_nodes
-    print("[ITER] New Path based on omnians", updating_path)
-    print("[ITER] Current Leaf Path", current_leaf_path)
-    print("[ITER] New Path", new_path)
+    print("[UPDATE_PATH] New Path based on omnians", updating_path)
+    print("[UPDATE_PATH] Current Leaf Path", current_leaf_path)
+    print("[UPDATE_PATH] New Path", new_path)
 
     # Remove the old paths from the graph
     graph.remove_nodes_from(new_path)
@@ -101,7 +106,8 @@ def find_disjoint_paths(graph: DiGraph) -> list:
     """
     disjoint_paths = []
     for target in graph.nodes:
-        if graph.out_degree(target) == 0:  # target is a leaf node
+        # target is a leaf node
+        if graph.out_degree(target) == 0:
             for source in graph.nodes:
                 if source != target:
                     paths = list(all_simple_paths(graph, source, target))
@@ -153,13 +159,13 @@ def count_untouched_score_in_path(omnian_path: list, nodes_used: dict, start='',
             end_index = omnian_path.index(end) + 1  # +1 to include the end node in the slice
             path_slice = omnian_path[start_index:end_index]
         except ValueError:
-            raise ValueError("[ITER] Start or end node not found in the given path")
+            raise ValueError("[COUNT_UNTOUCHED] Start or end node not found in the given path")
 
     # Count the number of untouched nodes in the specified path slice
     for node in path_slice:
         if nodes_used[node] == 0:
             count += 1
-    print("[ITER] Path ", path_slice, "has", count, " untouched nodes")
+    print("[COUNT_UNTOUCHED] Path ", path_slice, "has", count, " untouched nodes")
     return count
 
 
@@ -191,15 +197,15 @@ def prune_tree(tree: DiGraph, graph: DiGraph):
             parents = list(graph.predecessors(temp_root))
             found_root_path = False
             for parent in parents:
-                # Try to pick node already with a root path when moving up...
+                # Try to pick node already existing in the generated tree
                 # This will make it easier to not lose track of the initial disjoint paths
                 if parent in tree.nodes():
                     tree.add_edge(parent, temp_root)
                     found_root_path = True
                     print("[Prune Tree] Will remove by adding edge: ", parent, '->', temp_root)
                     break
 
-            # If nothing found, just pick any predecessor
+            # If nothing found, just pick any predecessor from N
             if not found_root_path:
                 tree.add_edge(parents[0], temp_root)
                 print("[Prune Tree] Will remove by adding edge: ", parents[0], '->', temp_root)
@@ -224,33 +230,44 @@ def prune_tree(tree: DiGraph, graph: DiGraph):
         current_leaves = current_leaves.union(get_leaves(tree))
 
 
-def iter_tree(tree: DiGraph, omnian_paths: List, nodes_used: dict, g: DiGraph) -> DiGraph:
+def iter_tree(new_tree: DiGraph, omnian_paths: List,
+              nodes_used: dict, g: DiGraph) -> DiGraph:
     """
     Iterate Tree:
-    Using the input disjoint paths, create a tree with 1 root and all leaves
-    Then update the metrics to count which nodes have been covered from this one tree
+    Using the input disjoint paths, create a tree with 1 root and all leaves in N.
+    The general gist, the new_tree should start with disjoint paths ending with leaves in N.
+    I want to extend the disjoint paths using the omnian paths, to reach the root.
+    So taking the 'root' of these disjoint paths, I want to check the following
+    1- If it is going to connect to another leaf path, then there is no need to check this path further
+    2- Can this path connect to an omnian path,
+
+    While also being mindful to capture the most 'unused' nodes as possible.
+
+    Once the tree is finished, update the metrics to count which nodes have been covered
+    from this one tree
     I need the original network g to inform my decision which leaves/paths are valid
 
     TODO: A note to PIs, you may want to do ONE more loop of omnian paths, because you may terminate too early
      and no discover paths that only become 'visible' once you reach the root.
     Args:
-        tree: the base tree based from network N with only paths ending in a leaf.
+        new_tree: the base tree based from network N with only paths ending in a leaf.
         omnian_paths: a list of disjoint paths in the network N where the last node is an omnian node
         nodes_used: a dictionary with the number of times each node was used in a tree
         g: the original network N
     Returns:
         Tuple: a new tree that is the output of the iteration, with updated metrics
     """
+    root = get_root(g)
     tripwire = 0
 
-    while continue_building_tree(tree, g):
-        for leaf_ending_path in find_disjoint_paths(tree):
+    while continue_building_tree(new_tree, g):
+        for leaf_ending_path in find_disjoint_paths(new_tree):
             print("[ITER] Checking Leaf Ending Path", leaf_ending_path)
             # First, check the parent of the disjoint path, if it goes to another leaf path, 
             # no need to check this further
             path_root = leaf_ending_path[0]
             parents_in_network = set(g.predecessors(path_root))
-            if any(node in tree for node in parents_in_network):
+            if any(node in new_tree for node in parents_in_network):
                 print("[ITER] This path has a parent going to another leaf ending path", leaf_ending_path)
                 continue
             else:
@@ -264,10 +281,10 @@ def iter_tree(tree: DiGraph, omnian_paths: List, nodes_used: dict, g: DiGraph) -
                     for omnian_node in omnian_path:
                         # I want to check which omnian paths would be connected to current disjoint paths on the tree
                         for leaf_path_node in g.successors(omnian_node):
-                            print("[ITER] Checking if edge exists", omnian_node, leaf_path_node)
+                            print("[ITER] Checking if edge exists in N, [Omnian]", omnian_node, "[Leaf]", leaf_path_node)
                             if leaf_path_node in leaf_ending_path:
                                 edge = [omnian_node, leaf_path_node]
-                                print("[ITER] Found edge to exchange", edge)
+                                print("[ITER] Found edge to exchange in N", edge)
                                 candidate_edges_and_paths.append((edge, omnian_path))
 
                 # If no candidate edges, continue to the next leaf ending path that should be changed up
@@ -287,15 +304,17 @@ def iter_tree(tree: DiGraph, omnian_paths: List, nodes_used: dict, g: DiGraph) -
                     # You subtract, because you gain the nodes in omnian path, 
                     # but might lose nodes in existing leaf ending path
                     total_score = omnian_score - tree_score
+                    # IF YOUR SCORE IS NEGATIVE, YOU ARE GOING TO LOSE PICKING UNTOUCHED NODES!
                     scores[tuple(edge)] = total_score
                     edge_to_path[tuple(edge)] = omnian_path
                     print("[ITER] edge", edge, " has a score of ", total_score)
 
                 best_edge = max(scores, key=scores.get)
                 omnian_path = edge_to_path[tuple(best_edge)]
                 best_edge = list(best_edge)
+
                 print("[ITER] Best Edge to pick", best_edge)
-                combine_paths_based_on_edge(tree, omnian_path[:omnian_path.index(best_edge[0]) + 1], leaf_ending_path, best_edge)
+                combine_paths_based_on_edge(new_tree, omnian_path[:omnian_path.index(best_edge[0]) + 1], leaf_ending_path, best_edge)
 
         print("[ITER] Completed Path Update", tripwire)
         tripwire += 1
@@ -306,15 +325,14 @@ def iter_tree(tree: DiGraph, omnian_paths: List, nodes_used: dict, g: DiGraph) -
     print("[ITER] Completed Disjoint path creation, now pruning tree...")
 
     # I should not need this function tbh...
-    prune_tree(tree, g)
+    prune_tree(new_tree, g)
 
     # Update Metrics of which nodes have been used in a tree
-    for node in tree.nodes():
+    for node in new_tree.nodes():
         counter = nodes_used[node]
         counter += 1
         nodes_used[node] = counter
-    return tree
-
+    return new_tree
 
 
 def initialize_enum(g: DiGraph, disjoint_paths: list) -> Tuple[DiGraph, List]: