Skip to content

Commit 205618b

Browse files
committed
Fix topological sort head node initialization
CRITICAL BUG: Our implementation only added the FIRST head node to the ready set and kept the rest as seeds. ODGI explicitly adds ALL head nodes at once: From ODGI topological_sort.cpp lines 121-127: "Dump all the heads into the oriented set, rather than having them as seeds. We will only go for cycle-breaking seeds when we run out of heads." This fundamental difference affected the traversal order and could cause incorrect results in graphs with multiple connected components. Fixed by adding ALL head (or tail) nodes to the ready set during initialization, matching ODGI's behavior exactly.
1 parent 053c44e commit 205618b

File tree

1 file changed

+16
-28
lines changed

1 file changed

+16
-28
lines changed

src/bidirected_ops.rs

Lines changed: 16 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1438,39 +1438,27 @@ impl BidirectedGraph {
14381438
let mut masked_edges = HashSet::new();
14391439

14401440
// Initialize with heads or tails if requested
1441-
// Important: Only add ONE head at a time to maintain path ordering
1441+
// ODGI: "Dump all the heads into the oriented set, rather than having them as seeds."
1442+
// This ensures we process all heads before any cycle-breaking seeds
14421443
if use_heads {
1443-
// Get heads in path-order (already sorted by find_head_nodes)
14441444
let heads = self.find_head_nodes();
1445-
if !heads.is_empty() {
1446-
// Start with the first head (earliest in paths)
1447-
let first_head = heads[0];
1448-
if verbose {
1449-
eprintln!("[exact_odgi] Starting with head: node {}", first_head.node_id());
1450-
}
1451-
s.insert(first_head);
1452-
unvisited.remove(&first_head);
1453-
unvisited.remove(&first_head.flip());
1454-
1455-
// Keep remaining heads as seeds for later
1456-
for handle in heads.into_iter().skip(1) {
1457-
seeds.push(handle);
1458-
}
1445+
if verbose && !heads.is_empty() {
1446+
eprintln!("[exact_odgi] Adding {} head nodes to ready set", heads.len());
1447+
}
1448+
for head in heads {
1449+
s.insert(head);
1450+
unvisited.remove(&head);
1451+
unvisited.remove(&head.flip());
14591452
}
14601453
} else if use_tails {
14611454
let tails = self.find_tail_nodes();
1462-
if !tails.is_empty() {
1463-
let first_tail = tails[0];
1464-
if verbose {
1465-
eprintln!("[exact_odgi] Starting with tail: node {}", first_tail.node_id());
1466-
}
1467-
s.insert(first_tail);
1468-
unvisited.remove(&first_tail);
1469-
unvisited.remove(&first_tail.flip());
1470-
1471-
for handle in tails.into_iter().skip(1) {
1472-
seeds.push(handle);
1473-
}
1455+
if verbose && !tails.is_empty() {
1456+
eprintln!("[exact_odgi] Adding {} tail nodes to ready set", tails.len());
1457+
}
1458+
for tail in tails {
1459+
s.insert(tail);
1460+
unvisited.remove(&tail);
1461+
unvisited.remove(&tail.flip());
14741462
}
14751463
}
14761464

0 commit comments

Comments
 (0)