Merge pull request #995 from benjeffery/fix-overflow

Error on cycles in found ancestor deps

Author: benjeffery

tests/test_ancestors.py

@@ -274,3 +274,25 @@ def test_grouping_random_cases(self, seed):
                     assert group_ids[anc_a] > group_ids[anc_b]
                 else:
                     assert group_ids[anc_a] < group_ids[anc_b]
+
+    def test_find_groups_cycle(self):
+        """
+        Test find_groups detects cycles in the dependency graph using a minimal example
+        """
+        # Create a direct cycle between two nodes where:
+        # Node 0 depends on node 1 (has incoming edge from 1)
+        # Node 1 depends on node 0 (has incoming edge from 0)
+        children_data = np.array(
+            [1, 0], dtype=np.int32
+        )  # Node 0 has child 1, node 1 has child 0
+        children_indices = np.array(
+            [0, 1, 2], dtype=np.int32
+        )  # Each node has one child
+        incoming_edge_count = np.array(
+            [1, 1], dtype=np.int32
+        )  # Each node has one incoming edge
+
+        with pytest.raises(
+            ValueError, match="Erroneous cycle in ancestor dependancies.*"
+        ):
+            ancestors.find_groups(children_data, children_indices, incoming_edge_count)

tsinfer/ancestors.py

@@ -133,6 +133,14 @@ def find_groups(children_data, children_indices, incoming_edge_count):
         # Add them to the group
         group_id[no_incoming] = current_group
         current_group += 1
+
+    # Check for unassigned nodes (cycles in dependency graph)
+    if np.any(group_id == -1):
+        raise ValueError(
+            "Erroneous cycle in ancestor dependancies, this is often "
+            "caused by too many unique site times. This fixed by discretising "
+            "the site times, for example rounding times to the nearest 0.1."
+        )
     return group_id