From dfa23270cf9126a00c7f0cdd86fbaf35001a21f0 Mon Sep 17 00:00:00 2001 From: rollrat Date: Sun, 31 May 2026 11:41:07 +0900 Subject: [PATCH 01/11] Refactor graph ids toward append-only allocation Keep graph node identifiers stable across world-to-logic transforms by removing compact id rebuilds and making petgraph conversion sparse-id safe. This prepares the graph model for provenance metadata without relying on node vector ordering. --- src/bin/check_nbt_world_cycle.rs | 5 +- src/graph/logic.rs | 11 +- src/graph/mod.rs | 302 ++++++++++++++++++++----------- src/transform/logic.rs | 46 +++-- src/transform/world.rs | 7 +- src/transform/world_to_logic.rs | 77 ++++++-- 6 files changed, 300 insertions(+), 148 deletions(-) diff --git a/src/bin/check_nbt_world_cycle.rs b/src/bin/check_nbt_world_cycle.rs index 5e103e8..ca99d5e 100644 --- a/src/bin/check_nbt_world_cycle.rs +++ b/src/bin/check_nbt_world_cycle.rs @@ -63,12 +63,13 @@ fn main() -> eyre::Result<()> { .map(|node| (node.id, node.kind.name())) .collect::>(); - let cyclic_components = kosaraju_scc(&graph.to_petgraph_only_edges()) + let petgraph = graph.to_petgraph_only_edges(); + let cyclic_components = kosaraju_scc(&petgraph) .into_iter() .map(|component| { component .into_iter() - .map(|node| node.index()) + .map(|node| petgraph[node]) .filter(|id| world_graph.positions.contains_key(id)) .collect::>() }) diff --git a/src/graph/logic.rs b/src/graph/logic.rs index 2ea0f55..7387f34 100644 --- a/src/graph/logic.rs +++ b/src/graph/logic.rs @@ -98,13 +98,13 @@ impl LogicGraph { where I: IntoIterator, { - let mut next_id = self.graph.max_node_id().map_or(0, |id| id + 1); + let mut next_id = self.graph.next_node_id(); for (name, source_id) in outputs { if self.find_node_by_id(source_id).is_none() { eyre::bail!("cannot attach output {name}: missing source node {source_id}"); } - self.graph.nodes.push(GraphNode { + self.graph.insert_node(GraphNode { id: next_id, kind: GraphNodeKind::Output(name), inputs: vec![source_id], @@ -113,7 +113,6 @@ impl LogicGraph { next_id += 1; } - self.graph.nodes.sort_by_key(|node| node.id); self.graph.build_outputs(); self.graph.build_producers(); self.graph.build_consumers(); @@ -780,8 +779,7 @@ mod tests { let splits = finish.graph.split_with_outputs(); - let mut graph: Graph = (&finish.graph.split_with_outputs()[0]).into(); - graph = graph.rebuild_node_ids(); + let graph: Graph = (&finish.graph.split_with_outputs()[0]).into(); println!("{}", graph.to_graphviz()); let mut transform = LogicGraphTransformer::new(LogicGraph { graph }); @@ -792,8 +790,7 @@ mod tests { #[allow(clippy::needless_range_loop)] for index in 1..2 { - let mut graph: Graph = (&splits[index]).into(); - graph = graph.rebuild_node_ids(); + let graph: Graph = (&splits[index]).into(); println!("{}", graph.to_graphviz()); let mut transform = LogicGraphTransformer::new(LogicGraph { graph }); diff --git a/src/graph/mod.rs b/src/graph/mod.rs index 082203a..b0326d5 100644 --- a/src/graph/mod.rs +++ b/src/graph/mod.rs @@ -154,15 +154,31 @@ pub struct Graph { } impl Graph { - pub fn to_petgraph_only_edges(&self) -> petgraph::Graph<(), ()> { - let edges = self.nodes.iter().flat_map(|node| { - node.outputs - .iter() - .map(|&id| (NodeIndex::new(node.id), NodeIndex::new(id))) - .collect_vec() - }); + fn to_petgraph_with_node_ids(&self) -> petgraph::Graph { + let mut graph = petgraph::Graph::::new(); + let mut id_to_index = HashMap::new(); + + for node in &self.nodes { + let index = graph.add_node(node.id); + id_to_index.insert(node.id, index); + } + + for node in &self.nodes { + let Some(source) = id_to_index.get(&node.id).copied() else { + continue; + }; + for output in &node.outputs { + if let Some(target) = id_to_index.get(output).copied() { + graph.add_edge(source, target, ()); + } + } + } - petgraph::Graph::<(), ()>::from_edges(edges) + graph + } + + pub fn to_petgraph_only_edges(&self) -> petgraph::Graph { + self.to_petgraph_with_node_ids() } pub fn to_petgraph(&self) -> petgraph::Graph { @@ -184,25 +200,24 @@ impl Graph { } pub fn topological_order(&self) -> Vec { - let nodes: HashSet = self.nodes.iter().map(|node| node.id).collect(); - petgraph::algo::toposort(&self.to_petgraph_only_edges(), None) + let graph = self.to_petgraph_only_edges(); + petgraph::algo::toposort(&graph, None) .unwrap() .iter() - .map(|index| index.index()) - .filter(|id| nodes.contains(id)) + .map(|index| graph[*index]) .collect_vec() } pub fn strongly_connected_components(&self) -> Vec> { let node_ids: HashSet = self.nodes.iter().map(|node| node.id).collect(); + let graph = self.to_petgraph_only_edges(); let mut seen = HashSet::new(); - let mut components = petgraph::algo::kosaraju_scc(&self.to_petgraph_only_edges()) + let mut components = petgraph::algo::kosaraju_scc(&graph) .into_iter() .filter_map(|component| { let mut component = component .into_iter() - .map(|index| index.index()) - .filter(|id| node_ids.contains(id)) + .map(|index| graph[index]) .collect_vec(); component.sort(); if component.is_empty() { @@ -250,10 +265,12 @@ impl Graph { &self, target_root: GraphNodeId, ) -> petgraph::algo::dominators::Dominators { - petgraph::algo::dominators::simple_fast( - &self.to_petgraph_only_edges(), - NodeIndex::new(target_root), - ) + let graph = self.to_petgraph_only_edges(); + let target_root = graph + .node_indices() + .find(|index| graph[*index] == target_root) + .expect("target root must exist in graph"); + petgraph::algo::dominators::simple_fast(&graph, target_root) } pub fn inputs(&self) -> Vec { @@ -517,6 +534,17 @@ impl Graph { .map(|node| node.id) } + pub fn next_node_id(&self) -> GraphNodeId { + self.max_node_id().map_or(0, |id| id + 1) + } + + pub fn insert_node(&mut self, node: GraphNode) { + match self.nodes.binary_search_by_key(&node.id, |node| node.id) { + Ok(index) => self.nodes[index] = node, + Err(index) => self.nodes.insert(index, node), + } + } + pub fn find_node_by_input_name(&mut self, input_name: &str) -> Option<&mut GraphNode> { self.nodes .iter_mut() @@ -530,27 +558,16 @@ impl Graph { } pub fn find_node_by_id(&self, node_id: GraphNodeId) -> Option<&GraphNode> { - // TODO: nodes is always sorted by id? - self.nodes - .binary_search_by_key(&node_id, |node| node.id) - .map(|index| &self.nodes[index]) - .ok() + self.nodes.iter().find(|node| node.id == node_id) } pub fn find_node_by_id_mut(&mut self, node_id: GraphNodeId) -> Option<&mut GraphNode> { - // TODO: nodes is always sorted by id? - self.nodes - .binary_search_by_key(&node_id, |node| node.id) - .map(|index| &mut self.nodes[index]) - .ok() + self.nodes.iter_mut().find(|node| node.id == node_id) } pub fn find_and_remove_node_by_id(&mut self, node_id: GraphNodeId) -> Option { - // TODO: nodes is always sorted by id? - self.nodes - .binary_search_by_key(&node_id, |node| node.id) - .map(|index| self.nodes.remove(index)) - .ok() + let index = self.nodes.iter().position(|node| node.id == node_id)?; + Some(self.nodes.remove(index)) } pub fn rebuild_node_id_base(&mut self, base_index: usize) { @@ -565,71 +582,8 @@ impl Graph { self.build_consumers(); } - pub fn rebuild_node_ids_deprecated(self) -> Self { - // toposort가 기존에 없는 새로운 index를 창조함 - // ??? - let index_map = petgraph::algo::toposort(&self.to_petgraph_only_edges(), None) - .unwrap() - .iter() - .enumerate() - .map(|index| (index.1.index(), index.0)) - .collect::>(); - - let mut nodes = self - .nodes - .into_iter() - .map(|node| GraphNode { - id: index_map[&node.id], - inputs: node.inputs.iter().map(|index| index_map[index]).collect(), - outputs: node.outputs.iter().map(|index| index_map[index]).collect(), - kind: node.kind, - tag: node.tag, - ..Default::default() - }) - .collect::>(); - nodes.sort_by_key(|node| node.id); - - let mut result = Self { nodes, ..self }; - result.build_producers(); - result.build_consumers(); - result - } - - pub fn rebuild_node_ids(mut self) -> Self { - self.nodes.sort_by_key(|node| match node.kind { - GraphNodeKind::Input(_) => 0, - GraphNodeKind::Output(_) => 1, - _ => 2, - }); - - let indexs: HashMap<_, _> = self - .nodes - .iter() - .enumerate() - .map(|(index, node)| (node.id, index)) - .collect(); - - let nodes = self - .nodes - .into_iter() - .map(|node| GraphNode { - id: indexs[&node.id], - inputs: node.inputs.iter().map(|index| indexs[index]).collect(), - outputs: node.outputs.iter().map(|index| indexs[index]).collect(), - kind: node.kind, - tag: node.tag, - ..Default::default() - }) - .collect::>(); - - let mut result = Self { nodes, ..self }; - result.build_producers(); - result.build_consumers(); - result - } - pub fn remove_by_node_id_lazy(&mut self, node_id: GraphNodeId) { - let Ok(index) = self.nodes.binary_search_by_key(&node_id, |node| node.id) else { + let Some(index) = self.nodes.iter().position(|node| node.id == node_id) else { return; }; @@ -644,7 +598,7 @@ impl Graph { outputs: Vec, tag: String, ) -> GraphNodeId { - let replacement_id = self.max_node_id().unwrap_or(0) + 1; + let replacement_id = self.next_node_id(); for input in &inputs { if let Some(node) = self.find_node_by_id_mut(*input) { @@ -666,14 +620,13 @@ impl Graph { self.remove_by_node_id_lazy(*node_id); } - self.nodes.push(GraphNode { + self.insert_node(GraphNode { id: replacement_id, kind, inputs, outputs, tag, }); - self.nodes.sort_by_key(|node| node.id); self.build_inputs(); self.build_outputs(); self.build_producers(); @@ -684,7 +637,7 @@ impl Graph { // 노드 삭제하고 삭제한 노드의 Input Output끼리 연결함 pub fn remove_and_reconnect_by_node_id_lazy(&mut self, node_id: GraphNodeId) { - let Ok(index) = self.nodes.binary_search_by_key(&node_id, |node| node.id) else { + let Some(index) = self.nodes.iter().position(|node| node.id == node_id) else { return; }; @@ -871,7 +824,7 @@ impl Graph { } pub fn critical_path(&self) -> Vec { - fn find_longest_path(graph: &petgraph::Graph<(), ()>) -> Option> { + fn find_longest_path(graph: &petgraph::Graph) -> Option> { let topo_order = petgraph::algo::toposort(&graph, None).unwrap(); let mut max_distances: Vec> = vec![None; graph.node_count()]; @@ -907,10 +860,11 @@ impl Graph { longest_path.cloned().flatten() } - let mut path = find_longest_path(&self.to_petgraph_only_edges()) + let graph = self.to_petgraph_only_edges(); + let mut path = find_longest_path(&graph) .unwrap() .iter() - .map(|id| id.index()) + .map(|id| graph[*id]) .collect_vec(); path.sort(); path @@ -947,10 +901,6 @@ impl Graph { } pub fn verify(&self) -> eyre::Result<()> { - if !self.nodes.windows(2).all(|w| w[0].id <= w[1].id) { - eyre::bail!("Nodes must be sorted by id!"); - } - let valid_ids: HashSet = self.nodes.iter().map(|node| node.id).collect(); for node in &self.nodes { @@ -1337,4 +1287,138 @@ mod tests { Ok(()) } + + #[test] + fn petgraph_conversion_does_not_materialize_sparse_node_ids() { + let graph = Graph { + nodes: vec![ + GraphNode { + id: 10, + outputs: vec![20], + ..Default::default() + }, + GraphNode { + id: 20, + inputs: vec![10], + ..Default::default() + }, + ], + ..Default::default() + }; + + let petgraph = graph.to_petgraph_only_edges(); + + assert_eq!(petgraph.node_count(), 2); + assert_eq!(petgraph.edge_count(), 1); + } + + #[test] + fn graph_algorithms_preserve_sparse_node_ids() { + let graph = Graph { + nodes: vec![ + GraphNode { + id: 10, + outputs: vec![20], + ..Default::default() + }, + GraphNode { + id: 20, + inputs: vec![10], + outputs: vec![30], + ..Default::default() + }, + GraphNode { + id: 30, + inputs: vec![20], + ..Default::default() + }, + ], + ..Default::default() + }; + + assert_eq!(graph.topological_order(), vec![10, 20, 30]); + assert_eq!( + graph.strongly_connected_components(), + vec![vec![10], vec![20], vec![30]] + ); + assert_eq!(graph.critical_path(), vec![10, 20, 30]); + } + + #[test] + fn next_node_id_is_append_only_for_sparse_graphs() { + let graph = Graph { + nodes: vec![ + GraphNode { + id: 10, + ..Default::default() + }, + GraphNode { + id: 30, + ..Default::default() + }, + ], + ..Default::default() + }; + + assert_eq!(graph.next_node_id(), 31); + } + + #[test] + fn insert_node_keeps_nodes_sorted_by_id() { + let mut graph = Graph { + nodes: vec![ + GraphNode { + id: 10, + ..Default::default() + }, + GraphNode { + id: 30, + ..Default::default() + }, + ], + ..Default::default() + }; + + graph.insert_node(GraphNode { + id: 20, + ..Default::default() + }); + + assert_eq!( + graph.nodes.iter().map(|node| node.id).collect_vec(), + vec![10, 20, 30] + ); + } + + #[test] + fn node_lookup_and_removal_do_not_require_sorted_nodes() { + let mut graph = Graph { + nodes: vec![ + GraphNode { + id: 30, + inputs: vec![10], + ..Default::default() + }, + GraphNode { + id: 10, + outputs: vec![30], + ..Default::default() + }, + GraphNode { + id: 20, + ..Default::default() + }, + ], + ..Default::default() + }; + + assert_eq!(graph.find_node_by_id(10).unwrap().id, 10); + graph.find_node_by_id_mut(20).unwrap().outputs = vec![30]; + assert_eq!(graph.find_node_by_id(20).unwrap().outputs, vec![30]); + assert_eq!(graph.find_and_remove_node_by_id(20).unwrap().id, 20); + + graph.verify().unwrap(); + graph.remove_by_node_id_lazy(10); + assert!(graph.find_node_by_id(10).is_none()); + } } diff --git a/src/transform/logic.rs b/src/transform/logic.rs index afd72ef..4a32c36 100644 --- a/src/transform/logic.rs +++ b/src/transform/logic.rs @@ -254,28 +254,45 @@ impl LogicGraphTransformer { } // optimize logic graph using quine mccluskey + let input_nodes = self + .graph + .graph + .nodes + .iter() + .filter(|node| matches!(node.kind, GraphNodeKind::Input(_))) + .cloned() + .collect_vec(); + let input_terms = input_nodes + .iter() + .enumerate() + .map(|(index, node)| (node.id, index as u8)) + .collect::>(); + fn make_qmc_form( graph: &Graph, + input_terms: &HashMap, node_id: GraphNodeId, ) -> eyre::Result { let node = graph.find_node_by_id(node_id).unwrap(); match &node.kind { - GraphNodeKind::Input(_) => Ok(quine_mc_cluskey::Bool::Term(node_id as u8)), - GraphNodeKind::Output(_) => make_qmc_form(graph, node.inputs[0]), + GraphNodeKind::Input(_) => Ok(quine_mc_cluskey::Bool::Term(input_terms[&node_id])), + GraphNodeKind::Output(_) => make_qmc_form(graph, input_terms, node.inputs[0]), GraphNodeKind::Logic(logic) => Ok(match &logic.logic_type { - LogicType::Not => { - quine_mc_cluskey::Bool::Not(Box::new(make_qmc_form(graph, node.inputs[0])?)) - } + LogicType::Not => quine_mc_cluskey::Bool::Not(Box::new(make_qmc_form( + graph, + input_terms, + node.inputs[0], + )?)), LogicType::And => quine_mc_cluskey::Bool::And( node.inputs .iter() - .map(|input| make_qmc_form(graph, *input).unwrap()) + .map(|input| make_qmc_form(graph, input_terms, *input).unwrap()) .collect(), ), LogicType::Or => quine_mc_cluskey::Bool::Or( node.inputs .iter() - .map(|input| make_qmc_form(graph, *input).unwrap()) + .map(|input| make_qmc_form(graph, input_terms, *input).unwrap()) .collect(), ), LogicType::Xor => unimplemented!(), @@ -284,7 +301,12 @@ impl LogicGraphTransformer { } } - let results = make_qmc_form(&self.graph.graph, self.graph.graph.outputs()[0])?.simplify(); + let results = make_qmc_form( + &self.graph.graph, + &input_terms, + self.graph.graph.outputs()[0], + )? + .simplify(); let mut nodes = Vec::new(); @@ -351,7 +373,7 @@ impl LogicGraphTransformer { } let mut id = 0; - let node = make_rc_form(&mut nodes, &mut id, &self.graph.graph.nodes, &results[0]); + let node = make_rc_form(&mut nodes, &mut id, &input_nodes, &results[0]); let output_node = GraphNode { id: nodes.len(), kind: GraphNodeKind::Output( @@ -466,7 +488,7 @@ impl LogicGraphTransformer { }) .collect_vec(); - let mut next_id = self.graph.graph.max_node_id().unwrap_or_default() + 1; + let mut next_id = self.graph.graph.next_node_id(); for (from, to) in direct_edges { let first_not = next_id; let second_not = next_id + 1; @@ -479,7 +501,7 @@ impl LogicGraphTransformer { .graph .replace_target_input_node_ids(to, from, vec![second_not]); - self.graph.graph.nodes.push(GraphNode { + self.graph.graph.insert_node(GraphNode { id: first_not, kind: GraphNodeKind::Logic(Logic { logic_type: LogicType::Not, @@ -488,7 +510,7 @@ impl LogicGraphTransformer { outputs: vec![second_not], tag: "auto-buffer".to_owned(), }); - self.graph.graph.nodes.push(GraphNode { + self.graph.graph.insert_node(GraphNode { id: second_not, kind: GraphNodeKind::Logic(Logic { logic_type: LogicType::Not, diff --git a/src/transform/world.rs b/src/transform/world.rs index db57a2b..5a9cc0e 100644 --- a/src/transform/world.rs +++ b/src/transform/world.rs @@ -84,7 +84,7 @@ impl WorldGraphTransformer { } // make clustered node - let mut next_id = self.graph.graph.max_node_id().unwrap(); + let mut next_id = self.graph.graph.next_node_id(); for members in group_members.values_mut() { members.sort_unstable(); } @@ -95,8 +95,6 @@ impl WorldGraphTransformer { } for group_id in group_ids.iter().sorted() { - next_id += 1; - let node = GraphNode { id: next_id, kind: GraphNodeKind::Block(Block { @@ -135,7 +133,8 @@ impl WorldGraphTransformer { }); self.graph.routings.insert(node.id); - self.graph.graph.nodes.push(node); + self.graph.graph.insert_node(node); + next_id += 1; } self.graph.graph.build_inputs(); diff --git a/src/transform/world_to_logic.rs b/src/transform/world_to_logic.rs index d23f52e..e10e91c 100644 --- a/src/transform/world_to_logic.rs +++ b/src/transform/world_to_logic.rs @@ -65,7 +65,7 @@ impl WorldToLogicTransformer { } pub fn transform(mut self) -> eyre::Result { - self.transform_inner(true) + self.transform_inner() } pub fn positions(&self) -> &HashMap { @@ -77,14 +77,14 @@ impl WorldToLogicTransformer { } pub fn transform_preserving_node_ids(mut self) -> eyre::Result { - self.transform_inner(false) + self.transform_inner() } - fn transform_inner(&mut self, rebuild_node_ids: bool) -> eyre::Result { + fn transform_inner(&mut self) -> eyre::Result { let mut new_in_id = HashMap::new(); let mut nodes = Vec::new(); - let mut next_new_id = self.graph.graph.max_node_id().unwrap(); + let mut next_new_id = self.graph.graph.next_node_id(); let mut input_count = 0; for id in self.graph.graph.topological_order() { @@ -143,8 +143,6 @@ impl WorldToLogicTransformer { tag, }] } else { - next_new_id += 1; - let or_node = GraphNode { id: node.id, kind: GraphNodeKind::Logic(Logic { @@ -166,6 +164,7 @@ impl WorldToLogicTransformer { }; new_in_id.insert(or_node.id, next_new_id); + next_new_id += 1; vec![or_node, not_node] } @@ -183,14 +182,10 @@ impl WorldToLogicTransformer { ..Default::default() }; - if rebuild_node_ids { - graph = graph.rebuild_node_ids(); - } else { - graph.nodes.sort_by_key(|node| node.id); - graph.build_outputs(); - graph.build_producers(); - graph.build_consumers(); - } + graph.nodes.sort_by_key(|node| node.id); + graph.build_outputs(); + graph.build_producers(); + graph.build_consumers(); Ok(LogicGraph { graph }) } @@ -361,4 +356,58 @@ mod tests { Ok(()) } + + #[test] + fn world_to_logic_transform_keeps_sparse_ids_and_appends_synthetic_nodes() -> eyre::Result<()> { + let switch = Block { + kind: BlockKind::Switch { is_on: false }, + direction: Direction::None, + }; + let torch = Block { + kind: BlockKind::Torch { is_on: true }, + direction: Direction::Bottom, + }; + let mut graph = crate::graph::Graph { + nodes: vec![ + crate::graph::GraphNode { + id: 10, + kind: crate::graph::GraphNodeKind::Block(switch), + outputs: vec![30], + ..Default::default() + }, + crate::graph::GraphNode { + id: 20, + kind: crate::graph::GraphNodeKind::Block(switch), + outputs: vec![30], + ..Default::default() + }, + crate::graph::GraphNode { + id: 30, + kind: crate::graph::GraphNodeKind::Block(torch), + inputs: vec![10, 20], + ..Default::default() + }, + ], + ..Default::default() + }; + graph.build_inputs(); + graph.build_outputs(); + graph.build_producers(); + graph.build_consumers(); + + let world_graph = crate::graph::world::WorldGraph { + graph, + ..Default::default() + }; + let logic = WorldToLogicTransformer::new(world_graph, true)?.transform()?; + + assert_eq!( + logic.graph.nodes.iter().map(|node| node.id).collect_vec(), + vec![10, 20, 30, 31] + ); + assert_eq!(logic.graph.find_node_by_id(30).unwrap().outputs, vec![31]); + assert_eq!(logic.graph.find_node_by_id(31).unwrap().inputs, vec![30]); + + Ok(()) + } } From 81d502d19a40f3ba7d240d80b447bee673f7c22a Mon Sep 17 00:00:00 2001 From: rollrat Date: Sun, 31 May 2026 11:53:35 +0900 Subject: [PATCH 02/11] Add monotonic graph node allocator Introduce a graph-owned node id counter and add coverage that newly allocated ids do not reuse removed node ids. This is the first step toward moving node identity out of GraphNode payloads. --- src/graph/mod.rs | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/src/graph/mod.rs b/src/graph/mod.rs index b0326d5..628c093 100644 --- a/src/graph/mod.rs +++ b/src/graph/mod.rs @@ -149,6 +149,7 @@ impl GraphNode { #[derive(Default, Debug, Clone)] pub struct Graph { pub nodes: Vec, + pub(crate) next_node_id: GraphNodeId, pub producers: HashMap>, pub consumers: HashMap>, } @@ -535,10 +536,25 @@ impl Graph { } pub fn next_node_id(&self) -> GraphNodeId { - self.max_node_id().map_or(0, |id| id + 1) + self.next_node_id + .max(self.max_node_id().map_or(0, |id| id + 1)) + } + + pub fn allocate_node_id(&mut self) -> GraphNodeId { + let id = self.next_node_id(); + self.next_node_id = id + 1; + id + } + + pub fn add_node(&mut self, mut node: GraphNode) -> GraphNodeId { + let id = self.allocate_node_id(); + node.id = id; + self.insert_node(node); + id } pub fn insert_node(&mut self, node: GraphNode) { + self.next_node_id = self.next_node_id.max(node.id + 1); match self.nodes.binary_search_by_key(&node.id, |node| node.id) { Ok(index) => self.nodes[index] = node, Err(index) => self.nodes.insert(index, node), @@ -577,6 +593,7 @@ impl Graph { *index += base_index; } } + self.next_node_id = self.next_node_id(); self.build_producers(); self.build_consumers(); @@ -1363,6 +1380,23 @@ mod tests { assert_eq!(graph.next_node_id(), 31); } + #[test] + fn graph_allocator_does_not_reuse_removed_ids() { + let mut graph = Graph::default(); + graph.insert_node(GraphNode { + id: 10, + ..Default::default() + }); + graph.remove_by_node_id_lazy(10); + + let id = graph.add_node(GraphNode::default()); + + assert_eq!(id, 11); + assert!(graph.find_node_by_id(10).is_none()); + assert!(graph.find_node_by_id(11).is_some()); + assert_eq!(graph.next_node_id(), 12); + } + #[test] fn insert_node_keeps_nodes_sorted_by_id() { let mut graph = Graph { From c2c429fd4280290009c43e1df4f5ac1c590270d2 Mon Sep 17 00:00:00 2001 From: rollrat Date: Sun, 31 May 2026 12:05:18 +0900 Subject: [PATCH 03/11] Store graph nodes by id Move Graph.nodes behind an id-keyed container so node ordering is structural instead of maintained by ad hoc sorts or binary-search insertion. This keeps synthetic ids append-only across removals during world-to-logic transforms and removes the transform-level manual sort. --- src/graph/graphviz.rs | 6 +- src/graph/logic.rs | 2 +- src/graph/mod.rs | 170 ++++++++++++++++-- src/graph/world.rs | 2 +- src/sequential/core.rs | 2 +- src/sequential/mod.rs | 6 +- src/transform/logic.rs | 5 +- .../place_and_route/local_placer/tests.rs | 15 +- src/transform/world_to_logic.rs | 15 +- 9 files changed, 184 insertions(+), 39 deletions(-) diff --git a/src/graph/graphviz.rs b/src/graph/graphviz.rs index 9989443..174ad1b 100644 --- a/src/graph/graphviz.rs +++ b/src/graph/graphviz.rs @@ -527,7 +527,8 @@ mod tests { kind: GraphNodeKind::None, tag: "Folded RS latch feedback SCC [1, 2, 3, 4]".to_owned(), ..Default::default() - }], + }] + .into(), ..Default::default() }; @@ -547,7 +548,8 @@ mod tests { kind: GraphNodeKind::None, tag: "debug source tag".to_owned(), ..Default::default() - }], + }] + .into(), ..Default::default() }; diff --git a/src/graph/logic.rs b/src/graph/logic.rs index 7387f34..d3b8f96 100644 --- a/src/graph/logic.rs +++ b/src/graph/logic.rs @@ -347,7 +347,7 @@ impl LogicGraphBuilder { self.do_parse(output_name); let mut graph = Graph { - nodes: self.nodes.clone(), + nodes: self.nodes.clone().into(), ..Default::default() }; graph.build_outputs(); diff --git a/src/graph/mod.rs b/src/graph/mod.rs index 628c093..8364fec 100644 --- a/src/graph/mod.rs +++ b/src/graph/mod.rs @@ -1,4 +1,4 @@ -use std::collections::{HashMap, HashSet, VecDeque}; +use std::collections::{BTreeMap, HashMap, HashSet, VecDeque}; use std::fmt::Display; use itertools::Itertools; @@ -146,9 +146,92 @@ impl GraphNode { } } +#[derive(Default, Debug, Clone)] +pub struct GraphNodes { + nodes: BTreeMap, +} + +impl GraphNodes { + pub fn len(&self) -> usize { + self.nodes.len() + } + + pub fn is_empty(&self) -> bool { + self.nodes.is_empty() + } + + pub fn iter(&self) -> std::collections::btree_map::Values<'_, GraphNodeId, GraphNode> { + self.nodes.values() + } + + pub fn iter_mut( + &mut self, + ) -> std::collections::btree_map::ValuesMut<'_, GraphNodeId, GraphNode> { + self.nodes.values_mut() + } + + pub fn retain(&mut self, f: impl FnMut(&GraphNode) -> bool) { + let mut f = f; + self.nodes.retain(|_, node| f(node)); + } + + pub fn remove(&mut self, index: usize) -> GraphNode { + let id = self + .nodes + .keys() + .nth(index) + .copied() + .expect("node index must exist"); + self.nodes.remove(&id).unwrap() + } + + pub fn extend(&mut self, nodes: impl IntoIterator) { + nodes.into_iter().for_each(|node| self.insert(node)); + } + + pub fn insert(&mut self, node: GraphNode) { + self.nodes.insert(node.id, node); + } +} + +impl From> for GraphNodes { + fn from(nodes: Vec) -> Self { + Self { + nodes: nodes.into_iter().map(|node| (node.id, node)).collect(), + } + } +} + +impl IntoIterator for GraphNodes { + type Item = GraphNode; + type IntoIter = std::collections::btree_map::IntoValues; + + fn into_iter(self) -> Self::IntoIter { + self.nodes.into_values() + } +} + +impl<'a> IntoIterator for &'a GraphNodes { + type Item = &'a GraphNode; + type IntoIter = std::collections::btree_map::Values<'a, GraphNodeId, GraphNode>; + + fn into_iter(self) -> Self::IntoIter { + self.nodes.values() + } +} + +impl<'a> IntoIterator for &'a mut GraphNodes { + type Item = &'a mut GraphNode; + type IntoIter = std::collections::btree_map::ValuesMut<'a, GraphNodeId, GraphNode>; + + fn into_iter(self) -> Self::IntoIter { + self.nodes.values_mut() + } +} + #[derive(Default, Debug, Clone)] pub struct Graph { - pub nodes: Vec, + pub nodes: GraphNodes, pub(crate) next_node_id: GraphNodeId, pub producers: HashMap>, pub consumers: HashMap>, @@ -540,6 +623,10 @@ impl Graph { .max(self.max_node_id().map_or(0, |id| id + 1)) } + fn preserve_next_node_id_high_watermark(&mut self) { + self.next_node_id = self.next_node_id(); + } + pub fn allocate_node_id(&mut self) -> GraphNodeId { let id = self.next_node_id(); self.next_node_id = id + 1; @@ -555,10 +642,7 @@ impl Graph { pub fn insert_node(&mut self, node: GraphNode) { self.next_node_id = self.next_node_id.max(node.id + 1); - match self.nodes.binary_search_by_key(&node.id, |node| node.id) { - Ok(index) => self.nodes[index] = node, - Err(index) => self.nodes.insert(index, node), - } + self.nodes.insert(node); } pub fn find_node_by_input_name(&mut self, input_name: &str) -> Option<&mut GraphNode> { @@ -582,6 +666,7 @@ impl Graph { } pub fn find_and_remove_node_by_id(&mut self, node_id: GraphNodeId) -> Option { + self.preserve_next_node_id_high_watermark(); let index = self.nodes.iter().position(|node| node.id == node_id)?; Some(self.nodes.remove(index)) } @@ -600,6 +685,7 @@ impl Graph { } pub fn remove_by_node_id_lazy(&mut self, node_id: GraphNodeId) { + self.preserve_next_node_id_high_watermark(); let Some(index) = self.nodes.iter().position(|node| node.id == node_id) else { return; }; @@ -654,6 +740,7 @@ impl Graph { // 노드 삭제하고 삭제한 노드의 Input Output끼리 연결함 pub fn remove_and_reconnect_by_node_id_lazy(&mut self, node_id: GraphNodeId) { + self.preserve_next_node_id_high_watermark(); let Some(index) = self.nodes.iter().position(|node| node.id == node_id) else { return; }; @@ -823,7 +910,7 @@ impl Graph { nodes.sort_by_key(|node| node.id); let mut graph = Self { - nodes, + nodes: nodes.into(), ..Default::default() }; graph.build_inputs(); @@ -892,6 +979,7 @@ impl Graph { } pub fn remove_input(&mut self, input_name: &str) { + self.preserve_next_node_id_high_watermark(); let Some((index, _)) = self.nodes.iter().find_position( |node| matches!(&node.kind, GraphNodeKind::Output(name) if name == input_name), ) else { @@ -905,6 +993,7 @@ impl Graph { } pub fn remove_output(&mut self, output_name: &str) -> Option { + self.preserve_next_node_id_high_watermark(); let (index, _) = self.nodes.iter().find_position( |node| matches!(&node.kind, GraphNodeKind::Output(name) if name == output_name), )?; @@ -1039,7 +1128,7 @@ impl From<&SubGraphWithGraph<'_>> for Graph { } let mut graph = Graph { - nodes, + nodes: nodes.into(), ..Default::default() }; graph.rebuild_node_id_base(0); @@ -1137,7 +1226,9 @@ pub fn subgraphs_to_clustered_graph(graph: &Graph, subgraphs: &[SubGraph]) -> Cl } let mut graph = Graph { - nodes: [nodes, weighted_node.into_iter().flatten().collect_vec()].concat(), + nodes: [nodes, weighted_node.into_iter().flatten().collect_vec()] + .concat() + .into(), ..Default::default() }; @@ -1178,7 +1269,8 @@ mod tests { inputs: vec![2, 0, 1], ..Default::default() }, - ], + ] + .into(), ..Default::default() }; @@ -1221,7 +1313,8 @@ mod tests { inputs: vec![2], ..Default::default() }, - ], + ] + .into(), ..Default::default() }; graph.build_producers(); @@ -1286,7 +1379,8 @@ mod tests { inputs: vec![2], ..Default::default() }, - ], + ] + .into(), ..Default::default() }; @@ -1319,7 +1413,8 @@ mod tests { inputs: vec![10], ..Default::default() }, - ], + ] + .into(), ..Default::default() }; @@ -1349,7 +1444,8 @@ mod tests { inputs: vec![20], ..Default::default() }, - ], + ] + .into(), ..Default::default() }; @@ -1373,13 +1469,51 @@ mod tests { id: 30, ..Default::default() }, - ], + ] + .into(), ..Default::default() }; assert_eq!(graph.next_node_id(), 31); } + #[test] + fn graph_nodes_are_keyed_by_id() { + let mut graph = Graph { + nodes: vec![ + GraphNode { + id: 30, + tag: "thirty".to_owned(), + ..Default::default() + }, + GraphNode { + id: 10, + tag: "ten".to_owned(), + ..Default::default() + }, + GraphNode { + id: 20, + tag: "twenty".to_owned(), + ..Default::default() + }, + ] + .into(), + ..Default::default() + }; + + graph.insert_node(GraphNode { + id: 20, + tag: "replacement".to_owned(), + ..Default::default() + }); + + assert_eq!( + graph.nodes.iter().map(|node| node.id).collect_vec(), + vec![10, 20, 30] + ); + assert_eq!(graph.find_node_by_id(20).unwrap().tag, "replacement"); + } + #[test] fn graph_allocator_does_not_reuse_removed_ids() { let mut graph = Graph::default(); @@ -1409,7 +1543,8 @@ mod tests { id: 30, ..Default::default() }, - ], + ] + .into(), ..Default::default() }; @@ -1442,7 +1577,8 @@ mod tests { id: 20, ..Default::default() }, - ], + ] + .into(), ..Default::default() }; diff --git a/src/graph/world.rs b/src/graph/world.rs index d34ae62..d49a0de 100644 --- a/src/graph/world.rs +++ b/src/graph/world.rs @@ -98,7 +98,7 @@ impl WorldGraphBuilder { let (nodes, positions) = self.build_nodes(); let mut graph = Graph { - nodes, + nodes: nodes.into(), ..Default::default() }; graph.build_inputs(); diff --git a/src/sequential/core.rs b/src/sequential/core.rs index 10a2480..692b9fc 100644 --- a/src/sequential/core.rs +++ b/src/sequential/core.rs @@ -126,7 +126,7 @@ pub fn rs_latch_prefix_graph(graph: &Graph, core: &RsLatchCore) -> Option Graph { inputs: vec![5], ..Default::default() }, - ], + ] + .into(), ..Default::default() }; graph.build_outputs(); @@ -251,7 +252,8 @@ fn d_latch_inner_graph() -> Graph { inputs: vec![8], ..Default::default() }, - ], + ] + .into(), ..Default::default() }; graph.build_outputs(); diff --git a/src/transform/logic.rs b/src/transform/logic.rs index 4a32c36..b7ad801 100644 --- a/src/transform/logic.rs +++ b/src/transform/logic.rs @@ -391,7 +391,7 @@ impl LogicGraphTransformer { nodes.push(output_node); nodes.sort_by_key(|node| node.id); - self.graph.graph.nodes = nodes; + self.graph.graph.nodes = nodes.into(); self.graph.graph.build_outputs(); self.graph.graph.build_producers(); self.graph.graph.build_consumers(); @@ -724,7 +724,8 @@ mod tests { inputs: vec![3], ..Default::default() }, - ], + ] + .into(), ..Default::default() }; graph.build_inputs(); diff --git a/src/transform/place_and_route/local_placer/tests.rs b/src/transform/place_and_route/local_placer/tests.rs index f9a69a0..25c6f93 100644 --- a/src/transform/place_and_route/local_placer/tests.rs +++ b/src/transform/place_and_route/local_placer/tests.rs @@ -394,7 +394,8 @@ fn future_join_cost_weights_pairs_with_remaining_fanout() { inputs: vec![3, 4], ..Default::default() }, - ], + ] + .into(), ..Default::default() }; graph.build_outputs(); @@ -564,7 +565,8 @@ fn ranked_sampling_preserves_diverse_geometry_candidates() -> eyre::Result<()> { (world.clone(), compact_a), (world.clone(), compact_b), (world, spread), - ], + ] + .into(), ); let sampled_b_positions = sampled .iter() @@ -823,7 +825,8 @@ fn local_placer_accepts_q_only_sequential_primitives_with_macro_candidates() { inputs: vec![2], ..Default::default() }, - ], + ] + .into(), ..Default::default() }; graph.build_inputs(); @@ -842,7 +845,8 @@ fn local_placer_rejects_multi_output_sequential_primitives_until_edges_are_port_ kind: GraphNodeKind::Sequential(SequentialPrimitive::rs_latch()), outputs: vec![1], ..Default::default() - }], + }] + .into(), ..Default::default() }; graph.build_inputs(); @@ -870,7 +874,8 @@ fn local_placer_rejects_sequential_primitives_without_macro_candidates() { )), outputs: vec![1], ..Default::default() - }], + }] + .into(), ..Default::default() }; graph.build_inputs(); diff --git a/src/transform/world_to_logic.rs b/src/transform/world_to_logic.rs index e10e91c..0961ebe 100644 --- a/src/transform/world_to_logic.rs +++ b/src/transform/world_to_logic.rs @@ -84,7 +84,6 @@ impl WorldToLogicTransformer { let mut new_in_id = HashMap::new(); let mut nodes = Vec::new(); - let mut next_new_id = self.graph.graph.next_node_id(); let mut input_count = 0; for id in self.graph.graph.topological_order() { @@ -143,18 +142,19 @@ impl WorldToLogicTransformer { tag, }] } else { + let not_node_id = self.graph.graph.allocate_node_id(); let or_node = GraphNode { id: node.id, kind: GraphNodeKind::Logic(Logic { logic_type: LogicType::Or, }), inputs, - outputs: vec![next_new_id], + outputs: vec![not_node_id], tag: tag.clone(), }; let not_node = GraphNode { - id: next_new_id, + id: not_node_id, kind: GraphNodeKind::Logic(Logic { logic_type: LogicType::Not, }), @@ -163,8 +163,7 @@ impl WorldToLogicTransformer { tag, }; - new_in_id.insert(or_node.id, next_new_id); - next_new_id += 1; + new_in_id.insert(or_node.id, not_node_id); vec![or_node, not_node] } @@ -178,11 +177,10 @@ impl WorldToLogicTransformer { } let mut graph = Graph { - nodes, + nodes: nodes.into(), ..Default::default() }; - graph.nodes.sort_by_key(|node| node.id); graph.build_outputs(); graph.build_producers(); graph.build_consumers(); @@ -387,7 +385,8 @@ mod tests { inputs: vec![10, 20], ..Default::default() }, - ], + ] + .into(), ..Default::default() }; graph.build_inputs(); From c0c24b3f45f8a1373ecace947a940f17c96b1649 Mon Sep 17 00:00:00 2001 From: rollrat Date: Sun, 31 May 2026 13:16:24 +0900 Subject: [PATCH 04/11] Initialize graph ids at construction Add Graph::from_nodes so imported graphs initialize next_node_id from existing node ids instead of patching allocator state during deletion. Update graph construction sites to use that API and remove the preserve_next_node_id_high_watermark deletion hook. --- src/graph/graphviz.rs | 28 +- src/graph/logic.rs | 5 +- src/graph/mod.rs | 424 ++++++++---------- src/graph/world.rs | 5 +- src/sequential/core.rs | 9 +- src/sequential/mod.rs | 280 ++++++------ src/transform/logic.rs | 88 ++-- .../place_and_route/local_placer/tests.rs | 211 ++++----- src/transform/world_to_logic.rs | 49 +- 9 files changed, 508 insertions(+), 591 deletions(-) diff --git a/src/graph/graphviz.rs b/src/graph/graphviz.rs index 174ad1b..91822a8 100644 --- a/src/graph/graphviz.rs +++ b/src/graph/graphviz.rs @@ -521,16 +521,12 @@ mod tests { #[test] fn table_graphviz_shows_node_tags() { - let graph = Graph { - nodes: vec![GraphNode { - id: 0, - kind: GraphNodeKind::None, - tag: "Folded RS latch feedback SCC [1, 2, 3, 4]".to_owned(), - ..Default::default() - }] - .into(), + let graph = Graph::from_nodes(vec![GraphNode { + id: 0, + kind: GraphNodeKind::None, + tag: "Folded RS latch feedback SCC [1, 2, 3, 4]".to_owned(), ..Default::default() - }; + }]); let dot = GraphvizBuilder::default() .with_graph(&graph) @@ -542,16 +538,12 @@ mod tests { #[test] fn graphviz_can_hide_node_tags() { - let graph = Graph { - nodes: vec![GraphNode { - id: 0, - kind: GraphNodeKind::None, - tag: "debug source tag".to_owned(), - ..Default::default() - }] - .into(), + let graph = Graph::from_nodes(vec![GraphNode { + id: 0, + kind: GraphNodeKind::None, + tag: "debug source tag".to_owned(), ..Default::default() - }; + }]); let dot = GraphvizBuilder::default() .with_graph(&graph) diff --git a/src/graph/logic.rs b/src/graph/logic.rs index d3b8f96..4a70fdb 100644 --- a/src/graph/logic.rs +++ b/src/graph/logic.rs @@ -346,10 +346,7 @@ impl LogicGraphBuilder { pub fn build(mut self, output_name: String) -> eyre::Result { self.do_parse(output_name); - let mut graph = Graph { - nodes: self.nodes.clone().into(), - ..Default::default() - }; + let mut graph = Graph::from_nodes(self.nodes.clone()); graph.build_outputs(); Ok(LogicGraph { graph }) diff --git a/src/graph/mod.rs b/src/graph/mod.rs index 8364fec..26698da 100644 --- a/src/graph/mod.rs +++ b/src/graph/mod.rs @@ -238,6 +238,19 @@ pub struct Graph { } impl Graph { + pub fn from_nodes(nodes: Vec) -> Self { + let next_node_id = nodes + .iter() + .map(|node| node.id) + .max() + .map_or(0, |id| id + 1); + Self { + nodes: nodes.into(), + next_node_id, + ..Default::default() + } + } + fn to_petgraph_with_node_ids(&self) -> petgraph::Graph { let mut graph = petgraph::Graph::::new(); let mut id_to_index = HashMap::new(); @@ -623,10 +636,6 @@ impl Graph { .max(self.max_node_id().map_or(0, |id| id + 1)) } - fn preserve_next_node_id_high_watermark(&mut self) { - self.next_node_id = self.next_node_id(); - } - pub fn allocate_node_id(&mut self) -> GraphNodeId { let id = self.next_node_id(); self.next_node_id = id + 1; @@ -666,7 +675,6 @@ impl Graph { } pub fn find_and_remove_node_by_id(&mut self, node_id: GraphNodeId) -> Option { - self.preserve_next_node_id_high_watermark(); let index = self.nodes.iter().position(|node| node.id == node_id)?; Some(self.nodes.remove(index)) } @@ -685,7 +693,6 @@ impl Graph { } pub fn remove_by_node_id_lazy(&mut self, node_id: GraphNodeId) { - self.preserve_next_node_id_high_watermark(); let Some(index) = self.nodes.iter().position(|node| node.id == node_id) else { return; }; @@ -740,7 +747,6 @@ impl Graph { // 노드 삭제하고 삭제한 노드의 Input Output끼리 연결함 pub fn remove_and_reconnect_by_node_id_lazy(&mut self, node_id: GraphNodeId) { - self.preserve_next_node_id_high_watermark(); let Some(index) = self.nodes.iter().position(|node| node.id == node_id) else { return; }; @@ -907,12 +913,7 @@ impl Graph { node.inputs.retain(|input| node_ids.contains(input)); node.outputs.retain(|output| node_ids.contains(output)); } - nodes.sort_by_key(|node| node.id); - - let mut graph = Self { - nodes: nodes.into(), - ..Default::default() - }; + let mut graph = Self::from_nodes(nodes); graph.build_inputs(); graph.build_outputs(); graph.build_producers(); @@ -979,7 +980,6 @@ impl Graph { } pub fn remove_input(&mut self, input_name: &str) { - self.preserve_next_node_id_high_watermark(); let Some((index, _)) = self.nodes.iter().find_position( |node| matches!(&node.kind, GraphNodeKind::Output(name) if name == input_name), ) else { @@ -993,7 +993,6 @@ impl Graph { } pub fn remove_output(&mut self, output_name: &str) -> Option { - self.preserve_next_node_id_high_watermark(); let (index, _) = self.nodes.iter().find_position( |node| matches!(&node.kind, GraphNodeKind::Output(name) if name == output_name), )?; @@ -1127,10 +1126,7 @@ impl From<&SubGraphWithGraph<'_>> for Graph { node.outputs.retain(|output| node_ids.contains(output)); } - let mut graph = Graph { - nodes: nodes.into(), - ..Default::default() - }; + let mut graph = Graph::from_nodes(nodes); graph.rebuild_node_id_base(0); graph @@ -1225,12 +1221,8 @@ pub fn subgraphs_to_clustered_graph(graph: &Graph, subgraphs: &[SubGraph]) -> Cl clustered_node.outputs = weighted_node[index].iter().map(|node| node.id).collect(); } - let mut graph = Graph { - nodes: [nodes, weighted_node.into_iter().flatten().collect_vec()] - .concat() - .into(), - ..Default::default() - }; + let mut graph = + Graph::from_nodes([nodes, weighted_node.into_iter().flatten().collect_vec()].concat()); graph.build_inputs(); graph.build_producers(); @@ -1247,32 +1239,28 @@ mod tests { #[test] fn build_inputs_and_outputs_are_sorted() { - let mut graph = Graph { - nodes: vec![ - GraphNode { - id: 0, - outputs: vec![3], - ..Default::default() - }, - GraphNode { - id: 1, - outputs: vec![3], - ..Default::default() - }, - GraphNode { - id: 2, - outputs: vec![3], - ..Default::default() - }, - GraphNode { - id: 3, - inputs: vec![2, 0, 1], - ..Default::default() - }, - ] - .into(), - ..Default::default() - }; + let mut graph = Graph::from_nodes(vec![ + GraphNode { + id: 0, + outputs: vec![3], + ..Default::default() + }, + GraphNode { + id: 1, + outputs: vec![3], + ..Default::default() + }, + GraphNode { + id: 2, + outputs: vec![3], + ..Default::default() + }, + GraphNode { + id: 3, + inputs: vec![2, 0, 1], + ..Default::default() + }, + ]); graph.build_inputs(); graph.build_outputs(); @@ -1285,38 +1273,34 @@ mod tests { #[test] fn extract_graph_by_node_ids_keeps_only_internal_edges() { - let mut graph = Graph { - nodes: vec![ - GraphNode { - id: 0, - outputs: vec![2], - ..Default::default() - }, - GraphNode { - id: 1, - outputs: vec![2], - ..Default::default() - }, - GraphNode { - id: 2, - inputs: vec![0, 1], - outputs: vec![3, 4], - ..Default::default() - }, - GraphNode { - id: 3, - inputs: vec![2], - ..Default::default() - }, - GraphNode { - id: 4, - inputs: vec![2], - ..Default::default() - }, - ] - .into(), - ..Default::default() - }; + let mut graph = Graph::from_nodes(vec![ + GraphNode { + id: 0, + outputs: vec![2], + ..Default::default() + }, + GraphNode { + id: 1, + outputs: vec![2], + ..Default::default() + }, + GraphNode { + id: 2, + inputs: vec![0, 1], + outputs: vec![3, 4], + ..Default::default() + }, + GraphNode { + id: 3, + inputs: vec![2], + ..Default::default() + }, + GraphNode { + id: 4, + inputs: vec![2], + ..Default::default() + }, + ]); graph.build_producers(); graph.build_consumers(); @@ -1348,41 +1332,37 @@ mod tests { #[test] fn sequential_node_kind_is_named_and_verified() -> eyre::Result<()> { - let mut graph = Graph { - nodes: vec![ - GraphNode { - id: 0, - kind: GraphNodeKind::Input("s".to_owned()), - outputs: vec![2], - ..Default::default() - }, - GraphNode { - id: 1, - kind: GraphNodeKind::Input("r".to_owned()), - outputs: vec![2], - ..Default::default() - }, - GraphNode { - id: 2, - kind: GraphNodeKind::Sequential(SequentialPrimitive::new( - SequentialType::RsLatch, - vec!["s".to_owned(), "r".to_owned()], - vec!["q".to_owned()], - )), - inputs: vec![0, 1], - outputs: vec![3], - ..Default::default() - }, - GraphNode { - id: 3, - kind: GraphNodeKind::Output("q".to_owned()), - inputs: vec![2], - ..Default::default() - }, - ] - .into(), - ..Default::default() - }; + let mut graph = Graph::from_nodes(vec![ + GraphNode { + id: 0, + kind: GraphNodeKind::Input("s".to_owned()), + outputs: vec![2], + ..Default::default() + }, + GraphNode { + id: 1, + kind: GraphNodeKind::Input("r".to_owned()), + outputs: vec![2], + ..Default::default() + }, + GraphNode { + id: 2, + kind: GraphNodeKind::Sequential(SequentialPrimitive::new( + SequentialType::RsLatch, + vec!["s".to_owned(), "r".to_owned()], + vec!["q".to_owned()], + )), + inputs: vec![0, 1], + outputs: vec![3], + ..Default::default() + }, + GraphNode { + id: 3, + kind: GraphNodeKind::Output("q".to_owned()), + inputs: vec![2], + ..Default::default() + }, + ]); graph.build_inputs(); graph.build_outputs(); @@ -1401,22 +1381,18 @@ mod tests { #[test] fn petgraph_conversion_does_not_materialize_sparse_node_ids() { - let graph = Graph { - nodes: vec![ - GraphNode { - id: 10, - outputs: vec![20], - ..Default::default() - }, - GraphNode { - id: 20, - inputs: vec![10], - ..Default::default() - }, - ] - .into(), - ..Default::default() - }; + let graph = Graph::from_nodes(vec![ + GraphNode { + id: 10, + outputs: vec![20], + ..Default::default() + }, + GraphNode { + id: 20, + inputs: vec![10], + ..Default::default() + }, + ]); let petgraph = graph.to_petgraph_only_edges(); @@ -1426,28 +1402,24 @@ mod tests { #[test] fn graph_algorithms_preserve_sparse_node_ids() { - let graph = Graph { - nodes: vec![ - GraphNode { - id: 10, - outputs: vec![20], - ..Default::default() - }, - GraphNode { - id: 20, - inputs: vec![10], - outputs: vec![30], - ..Default::default() - }, - GraphNode { - id: 30, - inputs: vec![20], - ..Default::default() - }, - ] - .into(), - ..Default::default() - }; + let graph = Graph::from_nodes(vec![ + GraphNode { + id: 10, + outputs: vec![20], + ..Default::default() + }, + GraphNode { + id: 20, + inputs: vec![10], + outputs: vec![30], + ..Default::default() + }, + GraphNode { + id: 30, + inputs: vec![20], + ..Default::default() + }, + ]); assert_eq!(graph.topological_order(), vec![10, 20, 30]); assert_eq!( @@ -1459,47 +1431,57 @@ mod tests { #[test] fn next_node_id_is_append_only_for_sparse_graphs() { - let graph = Graph { - nodes: vec![ - GraphNode { - id: 10, - ..Default::default() - }, - GraphNode { - id: 30, - ..Default::default() - }, - ] - .into(), - ..Default::default() - }; + let graph = Graph::from_nodes(vec![ + GraphNode { + id: 10, + ..Default::default() + }, + GraphNode { + id: 30, + ..Default::default() + }, + ]); assert_eq!(graph.next_node_id(), 31); } + #[test] + fn from_nodes_initializes_next_node_id_from_existing_ids() { + let mut graph = Graph::from_nodes(vec![ + GraphNode { + id: 10, + ..Default::default() + }, + GraphNode { + id: 30, + ..Default::default() + }, + ]); + + graph.remove_by_node_id_lazy(30); + + assert_eq!(graph.add_node(GraphNode::default()), 31); + } + #[test] fn graph_nodes_are_keyed_by_id() { - let mut graph = Graph { - nodes: vec![ - GraphNode { - id: 30, - tag: "thirty".to_owned(), - ..Default::default() - }, - GraphNode { - id: 10, - tag: "ten".to_owned(), - ..Default::default() - }, - GraphNode { - id: 20, - tag: "twenty".to_owned(), - ..Default::default() - }, - ] - .into(), - ..Default::default() - }; + let mut graph = Graph::from_nodes(vec![ + GraphNode { + id: 30, + tag: "thirty".to_owned(), + ..Default::default() + }, + GraphNode { + id: 10, + tag: "ten".to_owned(), + ..Default::default() + }, + GraphNode { + id: 20, + tag: "twenty".to_owned(), + ..Default::default() + }, + ]); graph.insert_node(GraphNode { id: 20, @@ -1533,20 +1515,16 @@ mod tests { #[test] fn insert_node_keeps_nodes_sorted_by_id() { - let mut graph = Graph { - nodes: vec![ - GraphNode { - id: 10, - ..Default::default() - }, - GraphNode { - id: 30, - ..Default::default() - }, - ] - .into(), - ..Default::default() - }; + let mut graph = Graph::from_nodes(vec![ + GraphNode { + id: 10, + ..Default::default() + }, + GraphNode { + id: 30, + ..Default::default() + }, + ]); graph.insert_node(GraphNode { id: 20, @@ -1561,26 +1539,22 @@ mod tests { #[test] fn node_lookup_and_removal_do_not_require_sorted_nodes() { - let mut graph = Graph { - nodes: vec![ - GraphNode { - id: 30, - inputs: vec![10], - ..Default::default() - }, - GraphNode { - id: 10, - outputs: vec![30], - ..Default::default() - }, - GraphNode { - id: 20, - ..Default::default() - }, - ] - .into(), - ..Default::default() - }; + let mut graph = Graph::from_nodes(vec![ + GraphNode { + id: 30, + inputs: vec![10], + ..Default::default() + }, + GraphNode { + id: 10, + outputs: vec![30], + ..Default::default() + }, + GraphNode { + id: 20, + ..Default::default() + }, + ]); assert_eq!(graph.find_node_by_id(10).unwrap().id, 10); graph.find_node_by_id_mut(20).unwrap().outputs = vec![30]; diff --git a/src/graph/world.rs b/src/graph/world.rs index d49a0de..5face67 100644 --- a/src/graph/world.rs +++ b/src/graph/world.rs @@ -97,10 +97,7 @@ impl WorldGraphBuilder { self.visit_blocks(); let (nodes, positions) = self.build_nodes(); - let mut graph = Graph { - nodes: nodes.into(), - ..Default::default() - }; + let mut graph = Graph::from_nodes(nodes); graph.build_inputs(); graph.build_producers(); diff --git a/src/sequential/core.rs b/src/sequential/core.rs index 692b9fc..d72e981 100644 --- a/src/sequential/core.rs +++ b/src/sequential/core.rs @@ -113,7 +113,7 @@ pub fn rs_latch_prefix_graph(graph: &Graph, core: &RsLatchCore) -> Option>(); - let mut next_node_id = graph.nodes.iter().map(|node| node.id).max().unwrap_or(0) + 1; + let mut next_node_id = graph.next_node_id(); for (output_name, source_node_id) in output_roots { nodes.push(GraphNode { id: next_node_id, @@ -123,12 +123,7 @@ pub fn rs_latch_prefix_graph(graph: &Graph, core: &RsLatchCore) -> Option Graph { fn rs_latch_inner_graph() -> Graph { // q = ~(r | nq), nq = ~(s | q) - let mut graph = Graph { - nodes: vec![ - GraphNode { - id: 0, - kind: GraphNodeKind::Input("s".to_owned()), - ..Default::default() - }, - GraphNode { - id: 1, - kind: GraphNodeKind::Input("r".to_owned()), - ..Default::default() - }, - GraphNode { - id: 2, - kind: GraphNodeKind::Logic(Logic { - logic_type: LogicType::Or, - }), - inputs: vec![1, 5], - ..Default::default() - }, - GraphNode { - id: 3, - kind: GraphNodeKind::Logic(Logic { - logic_type: LogicType::Not, - }), - inputs: vec![2], - ..Default::default() - }, - GraphNode { - id: 4, - kind: GraphNodeKind::Logic(Logic { - logic_type: LogicType::Or, - }), - inputs: vec![0, 3], - ..Default::default() - }, - GraphNode { - id: 5, - kind: GraphNodeKind::Logic(Logic { - logic_type: LogicType::Not, - }), - inputs: vec![4], - ..Default::default() - }, - GraphNode { - id: 6, - kind: GraphNodeKind::Output("q".to_owned()), - inputs: vec![3], - ..Default::default() - }, - GraphNode { - id: 7, - kind: GraphNodeKind::Output("nq".to_owned()), - inputs: vec![5], - ..Default::default() - }, - ] - .into(), - ..Default::default() - }; + let mut graph = Graph::from_nodes(vec![ + GraphNode { + id: 0, + kind: GraphNodeKind::Input("s".to_owned()), + ..Default::default() + }, + GraphNode { + id: 1, + kind: GraphNodeKind::Input("r".to_owned()), + ..Default::default() + }, + GraphNode { + id: 2, + kind: GraphNodeKind::Logic(Logic { + logic_type: LogicType::Or, + }), + inputs: vec![1, 5], + ..Default::default() + }, + GraphNode { + id: 3, + kind: GraphNodeKind::Logic(Logic { + logic_type: LogicType::Not, + }), + inputs: vec![2], + ..Default::default() + }, + GraphNode { + id: 4, + kind: GraphNodeKind::Logic(Logic { + logic_type: LogicType::Or, + }), + inputs: vec![0, 3], + ..Default::default() + }, + GraphNode { + id: 5, + kind: GraphNodeKind::Logic(Logic { + logic_type: LogicType::Not, + }), + inputs: vec![4], + ..Default::default() + }, + GraphNode { + id: 6, + kind: GraphNodeKind::Output("q".to_owned()), + inputs: vec![3], + ..Default::default() + }, + GraphNode { + id: 7, + kind: GraphNodeKind::Output("nq".to_owned()), + inputs: vec![5], + ..Default::default() + }, + ]); graph.build_outputs(); graph.build_producers(); graph.build_consumers(); @@ -172,90 +168,86 @@ fn rs_latch_inner_graph() -> Graph { fn d_latch_inner_graph() -> Graph { // s = d & en, r = ~d & en, q = ~(r | nq), nq = ~(s | q) - let mut graph = Graph { - nodes: vec![ - GraphNode { - id: 0, - kind: GraphNodeKind::Input("d".to_owned()), - ..Default::default() - }, - GraphNode { - id: 1, - kind: GraphNodeKind::Input("en".to_owned()), - ..Default::default() - }, - GraphNode { - id: 2, - kind: GraphNodeKind::Logic(Logic { - logic_type: LogicType::And, - }), - inputs: vec![0, 1], - ..Default::default() - }, - GraphNode { - id: 3, - kind: GraphNodeKind::Logic(Logic { - logic_type: LogicType::Not, - }), - inputs: vec![0], - ..Default::default() - }, - GraphNode { - id: 4, - kind: GraphNodeKind::Logic(Logic { - logic_type: LogicType::And, - }), - inputs: vec![3, 1], - ..Default::default() - }, - GraphNode { - id: 5, - kind: GraphNodeKind::Logic(Logic { - logic_type: LogicType::Or, - }), - inputs: vec![4, 8], - ..Default::default() - }, - GraphNode { - id: 6, - kind: GraphNodeKind::Logic(Logic { - logic_type: LogicType::Not, - }), - inputs: vec![5], - ..Default::default() - }, - GraphNode { - id: 7, - kind: GraphNodeKind::Logic(Logic { - logic_type: LogicType::Or, - }), - inputs: vec![2, 6], - ..Default::default() - }, - GraphNode { - id: 8, - kind: GraphNodeKind::Logic(Logic { - logic_type: LogicType::Not, - }), - inputs: vec![7], - ..Default::default() - }, - GraphNode { - id: 9, - kind: GraphNodeKind::Output("q".to_owned()), - inputs: vec![6], - ..Default::default() - }, - GraphNode { - id: 10, - kind: GraphNodeKind::Output("nq".to_owned()), - inputs: vec![8], - ..Default::default() - }, - ] - .into(), - ..Default::default() - }; + let mut graph = Graph::from_nodes(vec![ + GraphNode { + id: 0, + kind: GraphNodeKind::Input("d".to_owned()), + ..Default::default() + }, + GraphNode { + id: 1, + kind: GraphNodeKind::Input("en".to_owned()), + ..Default::default() + }, + GraphNode { + id: 2, + kind: GraphNodeKind::Logic(Logic { + logic_type: LogicType::And, + }), + inputs: vec![0, 1], + ..Default::default() + }, + GraphNode { + id: 3, + kind: GraphNodeKind::Logic(Logic { + logic_type: LogicType::Not, + }), + inputs: vec![0], + ..Default::default() + }, + GraphNode { + id: 4, + kind: GraphNodeKind::Logic(Logic { + logic_type: LogicType::And, + }), + inputs: vec![3, 1], + ..Default::default() + }, + GraphNode { + id: 5, + kind: GraphNodeKind::Logic(Logic { + logic_type: LogicType::Or, + }), + inputs: vec![4, 8], + ..Default::default() + }, + GraphNode { + id: 6, + kind: GraphNodeKind::Logic(Logic { + logic_type: LogicType::Not, + }), + inputs: vec![5], + ..Default::default() + }, + GraphNode { + id: 7, + kind: GraphNodeKind::Logic(Logic { + logic_type: LogicType::Or, + }), + inputs: vec![2, 6], + ..Default::default() + }, + GraphNode { + id: 8, + kind: GraphNodeKind::Logic(Logic { + logic_type: LogicType::Not, + }), + inputs: vec![7], + ..Default::default() + }, + GraphNode { + id: 9, + kind: GraphNodeKind::Output("q".to_owned()), + inputs: vec![6], + ..Default::default() + }, + GraphNode { + id: 10, + kind: GraphNodeKind::Output("nq".to_owned()), + inputs: vec![8], + ..Default::default() + }, + ]); graph.build_outputs(); graph.build_producers(); graph.build_consumers(); diff --git a/src/transform/logic.rs b/src/transform/logic.rs index b7ad801..b12ae6e 100644 --- a/src/transform/logic.rs +++ b/src/transform/logic.rs @@ -389,9 +389,7 @@ impl LogicGraphTransformer { ..Default::default() }; nodes.push(output_node); - nodes.sort_by_key(|node| node.id); - - self.graph.graph.nodes = nodes.into(); + self.graph.graph = Graph::from_nodes(nodes); self.graph.graph.build_outputs(); self.graph.graph.build_producers(); self.graph.graph.build_consumers(); @@ -684,50 +682,46 @@ mod tests { #[test] fn prepare_place_preserves_sequential_boundaries() -> eyre::Result<()> { - let mut graph = Graph { - nodes: vec![ - GraphNode { - id: 0, - kind: GraphNodeKind::Input("s".to_owned()), - outputs: vec![2], - ..Default::default() - }, - GraphNode { - id: 1, - kind: GraphNodeKind::Input("r".to_owned()), - outputs: vec![2], - ..Default::default() - }, - GraphNode { - id: 2, - kind: GraphNodeKind::Sequential(SequentialPrimitive::new( - SequentialType::RsLatch, - vec!["s".to_owned(), "r".to_owned()], - vec!["q".to_owned()], - )), - inputs: vec![0, 1], - outputs: vec![3], - ..Default::default() - }, - GraphNode { - id: 3, - kind: GraphNodeKind::Logic(Logic { - logic_type: LogicType::Not, - }), - inputs: vec![2], - outputs: vec![4], - ..Default::default() - }, - GraphNode { - id: 4, - kind: GraphNodeKind::Output("not_q".to_owned()), - inputs: vec![3], - ..Default::default() - }, - ] - .into(), - ..Default::default() - }; + let mut graph = Graph::from_nodes(vec![ + GraphNode { + id: 0, + kind: GraphNodeKind::Input("s".to_owned()), + outputs: vec![2], + ..Default::default() + }, + GraphNode { + id: 1, + kind: GraphNodeKind::Input("r".to_owned()), + outputs: vec![2], + ..Default::default() + }, + GraphNode { + id: 2, + kind: GraphNodeKind::Sequential(SequentialPrimitive::new( + SequentialType::RsLatch, + vec!["s".to_owned(), "r".to_owned()], + vec!["q".to_owned()], + )), + inputs: vec![0, 1], + outputs: vec![3], + ..Default::default() + }, + GraphNode { + id: 3, + kind: GraphNodeKind::Logic(Logic { + logic_type: LogicType::Not, + }), + inputs: vec![2], + outputs: vec![4], + ..Default::default() + }, + GraphNode { + id: 4, + kind: GraphNodeKind::Output("not_q".to_owned()), + inputs: vec![3], + ..Default::default() + }, + ]); graph.build_inputs(); graph.build_outputs(); diff --git a/src/transform/place_and_route/local_placer/tests.rs b/src/transform/place_and_route/local_placer/tests.rs index 25c6f93..35aa5f6 100644 --- a/src/transform/place_and_route/local_placer/tests.rs +++ b/src/transform/place_and_route/local_placer/tests.rs @@ -343,61 +343,57 @@ fn generate_with_outputs_reports_materialized_output_positions() -> eyre::Result #[test] fn future_join_cost_weights_pairs_with_remaining_fanout() { - let mut graph = Graph { - nodes: vec![ - GraphNode { - id: 0, - kind: GraphNodeKind::Input("a".to_owned()), - ..Default::default() - }, - GraphNode { - id: 1, - kind: GraphNodeKind::Input("b".to_owned()), - ..Default::default() - }, - GraphNode { - id: 2, - kind: GraphNodeKind::Input("c".to_owned()), - ..Default::default() - }, - GraphNode { - id: 3, - kind: GraphNodeKind::Input("d".to_owned()), - ..Default::default() - }, - GraphNode { - id: 4, - kind: GraphNodeKind::Input("e".to_owned()), - ..Default::default() - }, - GraphNode { - id: 5, - kind: GraphNodeKind::Logic(crate::logic::Logic { - logic_type: LogicType::Or, - }), - inputs: vec![0, 1], - ..Default::default() - }, - GraphNode { - id: 6, - kind: GraphNodeKind::Logic(crate::logic::Logic { - logic_type: LogicType::Or, - }), - inputs: vec![0, 2], - ..Default::default() - }, - GraphNode { - id: 7, - kind: GraphNodeKind::Logic(crate::logic::Logic { - logic_type: LogicType::Or, - }), - inputs: vec![3, 4], - ..Default::default() - }, - ] - .into(), - ..Default::default() - }; + let mut graph = Graph::from_nodes(vec![ + GraphNode { + id: 0, + kind: GraphNodeKind::Input("a".to_owned()), + ..Default::default() + }, + GraphNode { + id: 1, + kind: GraphNodeKind::Input("b".to_owned()), + ..Default::default() + }, + GraphNode { + id: 2, + kind: GraphNodeKind::Input("c".to_owned()), + ..Default::default() + }, + GraphNode { + id: 3, + kind: GraphNodeKind::Input("d".to_owned()), + ..Default::default() + }, + GraphNode { + id: 4, + kind: GraphNodeKind::Input("e".to_owned()), + ..Default::default() + }, + GraphNode { + id: 5, + kind: GraphNodeKind::Logic(crate::logic::Logic { + logic_type: LogicType::Or, + }), + inputs: vec![0, 1], + ..Default::default() + }, + GraphNode { + id: 6, + kind: GraphNodeKind::Logic(crate::logic::Logic { + logic_type: LogicType::Or, + }), + inputs: vec![0, 2], + ..Default::default() + }, + GraphNode { + id: 7, + kind: GraphNodeKind::Logic(crate::logic::Logic { + logic_type: LogicType::Or, + }), + inputs: vec![3, 4], + ..Default::default() + }, + ]); graph.build_outputs(); let graph = LogicGraph { graph }; let visit_orders = vec![0, 1, 2, 3, 4, 5, 6, 7]; @@ -565,8 +561,7 @@ fn ranked_sampling_preserves_diverse_geometry_candidates() -> eyre::Result<()> { (world.clone(), compact_a), (world.clone(), compact_b), (world, spread), - ] - .into(), + ], ); let sampled_b_positions = sampled .iter() @@ -794,41 +789,37 @@ fn generate_or_routes_finds_adjacent_torch_route() { #[test] fn local_placer_accepts_q_only_sequential_primitives_with_macro_candidates() { - let mut graph = Graph { - nodes: vec![ - GraphNode { - id: 0, - kind: GraphNodeKind::Input("s".to_owned()), - outputs: vec![2], - ..Default::default() - }, - GraphNode { - id: 1, - kind: GraphNodeKind::Input("r".to_owned()), - outputs: vec![2], - ..Default::default() - }, - GraphNode { - id: 2, - kind: GraphNodeKind::Sequential(SequentialPrimitive::new( - SequentialType::RsLatch, - vec!["s".to_owned(), "r".to_owned()], - vec!["q".to_owned()], - )), - inputs: vec![0, 1], - outputs: vec![3], - ..Default::default() - }, - GraphNode { - id: 3, - kind: GraphNodeKind::Output("q".to_owned()), - inputs: vec![2], - ..Default::default() - }, - ] - .into(), - ..Default::default() - }; + let mut graph = Graph::from_nodes(vec![ + GraphNode { + id: 0, + kind: GraphNodeKind::Input("s".to_owned()), + outputs: vec![2], + ..Default::default() + }, + GraphNode { + id: 1, + kind: GraphNodeKind::Input("r".to_owned()), + outputs: vec![2], + ..Default::default() + }, + GraphNode { + id: 2, + kind: GraphNodeKind::Sequential(SequentialPrimitive::new( + SequentialType::RsLatch, + vec!["s".to_owned(), "r".to_owned()], + vec!["q".to_owned()], + )), + inputs: vec![0, 1], + outputs: vec![3], + ..Default::default() + }, + GraphNode { + id: 3, + kind: GraphNodeKind::Output("q".to_owned()), + inputs: vec![2], + ..Default::default() + }, + ]); graph.build_inputs(); graph.build_outputs(); @@ -839,16 +830,12 @@ fn local_placer_accepts_q_only_sequential_primitives_with_macro_candidates() { #[test] fn local_placer_rejects_multi_output_sequential_primitives_until_edges_are_port_aware() { - let mut graph = Graph { - nodes: vec![GraphNode { - id: 0, - kind: GraphNodeKind::Sequential(SequentialPrimitive::rs_latch()), - outputs: vec![1], - ..Default::default() - }] - .into(), + let mut graph = Graph::from_nodes(vec![GraphNode { + id: 0, + kind: GraphNodeKind::Sequential(SequentialPrimitive::rs_latch()), + outputs: vec![1], ..Default::default() - }; + }]); graph.build_inputs(); graph.build_outputs(); @@ -864,20 +851,16 @@ fn local_placer_rejects_multi_output_sequential_primitives_until_edges_are_port_ #[test] fn local_placer_rejects_sequential_primitives_without_macro_candidates() { - let mut graph = Graph { - nodes: vec![GraphNode { - id: 0, - kind: GraphNodeKind::Sequential(SequentialPrimitive::new( - SequentialType::DFlipFlop, - Vec::new(), - vec!["q".to_owned()], - )), - outputs: vec![1], - ..Default::default() - }] - .into(), + let mut graph = Graph::from_nodes(vec![GraphNode { + id: 0, + kind: GraphNodeKind::Sequential(SequentialPrimitive::new( + SequentialType::DFlipFlop, + Vec::new(), + vec!["q".to_owned()], + )), + outputs: vec![1], ..Default::default() - }; + }]); graph.build_inputs(); graph.build_outputs(); diff --git a/src/transform/world_to_logic.rs b/src/transform/world_to_logic.rs index 0961ebe..49f3f67 100644 --- a/src/transform/world_to_logic.rs +++ b/src/transform/world_to_logic.rs @@ -176,10 +176,7 @@ impl WorldToLogicTransformer { nodes.extend(new_nodes); } - let mut graph = Graph { - nodes: nodes.into(), - ..Default::default() - }; + let mut graph = Graph::from_nodes(nodes); graph.build_outputs(); graph.build_producers(); @@ -365,30 +362,26 @@ mod tests { kind: BlockKind::Torch { is_on: true }, direction: Direction::Bottom, }; - let mut graph = crate::graph::Graph { - nodes: vec![ - crate::graph::GraphNode { - id: 10, - kind: crate::graph::GraphNodeKind::Block(switch), - outputs: vec![30], - ..Default::default() - }, - crate::graph::GraphNode { - id: 20, - kind: crate::graph::GraphNodeKind::Block(switch), - outputs: vec![30], - ..Default::default() - }, - crate::graph::GraphNode { - id: 30, - kind: crate::graph::GraphNodeKind::Block(torch), - inputs: vec![10, 20], - ..Default::default() - }, - ] - .into(), - ..Default::default() - }; + let mut graph = crate::graph::Graph::from_nodes(vec![ + crate::graph::GraphNode { + id: 10, + kind: crate::graph::GraphNodeKind::Block(switch), + outputs: vec![30], + ..Default::default() + }, + crate::graph::GraphNode { + id: 20, + kind: crate::graph::GraphNodeKind::Block(switch), + outputs: vec![30], + ..Default::default() + }, + crate::graph::GraphNode { + id: 30, + kind: crate::graph::GraphNodeKind::Block(torch), + inputs: vec![10, 20], + ..Default::default() + }, + ]); graph.build_inputs(); graph.build_outputs(); graph.build_producers(); From f2e0c088585cc279af551d77f528350f66973083 Mon Sep 17 00:00:00 2001 From: rollrat Date: Sun, 31 May 2026 13:26:49 +0900 Subject: [PATCH 05/11] Consolidate graph id tests Collapse overlapping sparse-id, id-keyed storage, and append-only allocator tests into one focused graph regression while keeping the world-to-logic sparse synthetic-node test. This reduces test fixture bulk without dropping the core invariants. --- src/graph/mod.rs | 169 +++++------------------------------------------ 1 file changed, 18 insertions(+), 151 deletions(-) diff --git a/src/graph/mod.rs b/src/graph/mod.rs index 26698da..beb7d2c 100644 --- a/src/graph/mod.rs +++ b/src/graph/mod.rs @@ -1380,29 +1380,13 @@ mod tests { } #[test] - fn petgraph_conversion_does_not_materialize_sparse_node_ids() { - let graph = Graph::from_nodes(vec![ - GraphNode { - id: 10, - outputs: vec![20], - ..Default::default() - }, + fn sparse_node_ids_are_keyed_and_append_only() { + let mut graph = Graph::from_nodes(vec![ GraphNode { - id: 20, - inputs: vec![10], + id: 30, + inputs: vec![20], ..Default::default() }, - ]); - - let petgraph = graph.to_petgraph_only_edges(); - - assert_eq!(petgraph.node_count(), 2); - assert_eq!(petgraph.edge_count(), 1); - } - - #[test] - fn graph_algorithms_preserve_sparse_node_ids() { - let graph = Graph::from_nodes(vec![ GraphNode { id: 10, outputs: vec![20], @@ -1414,155 +1398,38 @@ mod tests { outputs: vec![30], ..Default::default() }, - GraphNode { - id: 30, - inputs: vec![20], - ..Default::default() - }, ]); + assert_eq!( + graph.nodes.iter().map(|node| node.id).collect_vec(), + vec![10, 20, 30] + ); assert_eq!(graph.topological_order(), vec![10, 20, 30]); assert_eq!( graph.strongly_connected_components(), vec![vec![10], vec![20], vec![30]] ); assert_eq!(graph.critical_path(), vec![10, 20, 30]); - } - - #[test] - fn next_node_id_is_append_only_for_sparse_graphs() { - let graph = Graph::from_nodes(vec![ - GraphNode { - id: 10, - ..Default::default() - }, - GraphNode { - id: 30, - ..Default::default() - }, - ]); - - assert_eq!(graph.next_node_id(), 31); - } - - #[test] - fn from_nodes_initializes_next_node_id_from_existing_ids() { - let mut graph = Graph::from_nodes(vec![ - GraphNode { - id: 10, - ..Default::default() - }, - GraphNode { - id: 30, - ..Default::default() - }, - ]); - - graph.remove_by_node_id_lazy(30); - - assert_eq!(graph.add_node(GraphNode::default()), 31); - } - - #[test] - fn graph_nodes_are_keyed_by_id() { - let mut graph = Graph::from_nodes(vec![ - GraphNode { - id: 30, - tag: "thirty".to_owned(), - ..Default::default() - }, - GraphNode { - id: 10, - tag: "ten".to_owned(), - ..Default::default() - }, - GraphNode { - id: 20, - tag: "twenty".to_owned(), - ..Default::default() - }, - ]); + let petgraph = graph.to_petgraph_only_edges(); + assert_eq!(petgraph.node_count(), 3); + assert_eq!(petgraph.edge_count(), 2); graph.insert_node(GraphNode { - id: 20, + id: 25, tag: "replacement".to_owned(), ..Default::default() }); - assert_eq!( graph.nodes.iter().map(|node| node.id).collect_vec(), - vec![10, 20, 30] + vec![10, 20, 25, 30] ); - assert_eq!(graph.find_node_by_id(20).unwrap().tag, "replacement"); - } - - #[test] - fn graph_allocator_does_not_reuse_removed_ids() { - let mut graph = Graph::default(); - graph.insert_node(GraphNode { - id: 10, - ..Default::default() - }); - graph.remove_by_node_id_lazy(10); + assert_eq!(graph.find_node_by_id(25).unwrap().tag, "replacement"); + assert_eq!(graph.find_and_remove_node_by_id(25).unwrap().id, 25); + graph.remove_by_node_id_lazy(30); let id = graph.add_node(GraphNode::default()); - assert_eq!(id, 11); - assert!(graph.find_node_by_id(10).is_none()); - assert!(graph.find_node_by_id(11).is_some()); - assert_eq!(graph.next_node_id(), 12); - } - - #[test] - fn insert_node_keeps_nodes_sorted_by_id() { - let mut graph = Graph::from_nodes(vec![ - GraphNode { - id: 10, - ..Default::default() - }, - GraphNode { - id: 30, - ..Default::default() - }, - ]); - - graph.insert_node(GraphNode { - id: 20, - ..Default::default() - }); - - assert_eq!( - graph.nodes.iter().map(|node| node.id).collect_vec(), - vec![10, 20, 30] - ); - } - - #[test] - fn node_lookup_and_removal_do_not_require_sorted_nodes() { - let mut graph = Graph::from_nodes(vec![ - GraphNode { - id: 30, - inputs: vec![10], - ..Default::default() - }, - GraphNode { - id: 10, - outputs: vec![30], - ..Default::default() - }, - GraphNode { - id: 20, - ..Default::default() - }, - ]); - - assert_eq!(graph.find_node_by_id(10).unwrap().id, 10); - graph.find_node_by_id_mut(20).unwrap().outputs = vec![30]; - assert_eq!(graph.find_node_by_id(20).unwrap().outputs, vec![30]); - assert_eq!(graph.find_and_remove_node_by_id(20).unwrap().id, 20); - - graph.verify().unwrap(); - graph.remove_by_node_id_lazy(10); - assert!(graph.find_node_by_id(10).is_none()); + assert_eq!(id, 31); + assert!(graph.find_node_by_id(31).is_some()); } } From c40d393ab1038ffba584c4f2f5bd918ef7bc5255 Mon Sep 17 00:00:00 2001 From: rollrat Date: Sun, 31 May 2026 13:57:15 +0900 Subject: [PATCH 06/11] Use IndexMap for graph node storage Switch the graph node storage wrapper from BTreeMap to IndexMap while preserving deterministic id-ordered iteration at construction and insertion. This keeps the storage ready for graph-owned node ids without changing traversal output. --- Cargo.lock | 1 + Cargo.toml | 1 + src/graph/mod.rs | 32 +++++++++++++++----------------- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2440605..d1fe415 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -683,6 +683,7 @@ dependencies = [ "fastanvil", "fastnbt", "flate2", + "indexmap 2.0.0", "indicatif", "itertools", "mimalloc", diff --git a/Cargo.toml b/Cargo.toml index a0e6699..28b85f5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,7 @@ eyre = "0.6.8" fastnbt = "2" fastanvil = "0.26" indicatif = { version = "0.17.9", features = ["rayon"] } +indexmap = "2.0.0" itertools = "0.11.0" petgraph = "0.6.3" quine-mc_cluskey = "0.2.4" diff --git a/src/graph/mod.rs b/src/graph/mod.rs index beb7d2c..2c11cf2 100644 --- a/src/graph/mod.rs +++ b/src/graph/mod.rs @@ -1,6 +1,7 @@ -use std::collections::{BTreeMap, HashMap, HashSet, VecDeque}; +use std::collections::{HashMap, HashSet, VecDeque}; use std::fmt::Display; +use indexmap::IndexMap; use itertools::Itertools; use module::GraphModuleBuilder; use petgraph::stable_graph::NodeIndex; @@ -148,7 +149,7 @@ impl GraphNode { #[derive(Default, Debug, Clone)] pub struct GraphNodes { - nodes: BTreeMap, + nodes: IndexMap, } impl GraphNodes { @@ -160,13 +161,11 @@ impl GraphNodes { self.nodes.is_empty() } - pub fn iter(&self) -> std::collections::btree_map::Values<'_, GraphNodeId, GraphNode> { + pub fn iter(&self) -> indexmap::map::Values<'_, GraphNodeId, GraphNode> { self.nodes.values() } - pub fn iter_mut( - &mut self, - ) -> std::collections::btree_map::ValuesMut<'_, GraphNodeId, GraphNode> { + pub fn iter_mut(&mut self) -> indexmap::map::ValuesMut<'_, GraphNodeId, GraphNode> { self.nodes.values_mut() } @@ -176,13 +175,10 @@ impl GraphNodes { } pub fn remove(&mut self, index: usize) -> GraphNode { - let id = self - .nodes - .keys() - .nth(index) - .copied() - .expect("node index must exist"); - self.nodes.remove(&id).unwrap() + self.nodes + .shift_remove_index(index) + .map(|(_, node)| node) + .expect("node index must exist") } pub fn extend(&mut self, nodes: impl IntoIterator) { @@ -191,11 +187,13 @@ impl GraphNodes { pub fn insert(&mut self, node: GraphNode) { self.nodes.insert(node.id, node); + self.nodes.sort_by(|id, _, other_id, _| id.cmp(other_id)); } } impl From> for GraphNodes { - fn from(nodes: Vec) -> Self { + fn from(mut nodes: Vec) -> Self { + nodes.sort_by_key(|node| node.id); Self { nodes: nodes.into_iter().map(|node| (node.id, node)).collect(), } @@ -204,7 +202,7 @@ impl From> for GraphNodes { impl IntoIterator for GraphNodes { type Item = GraphNode; - type IntoIter = std::collections::btree_map::IntoValues; + type IntoIter = indexmap::map::IntoValues; fn into_iter(self) -> Self::IntoIter { self.nodes.into_values() @@ -213,7 +211,7 @@ impl IntoIterator for GraphNodes { impl<'a> IntoIterator for &'a GraphNodes { type Item = &'a GraphNode; - type IntoIter = std::collections::btree_map::Values<'a, GraphNodeId, GraphNode>; + type IntoIter = indexmap::map::Values<'a, GraphNodeId, GraphNode>; fn into_iter(self) -> Self::IntoIter { self.nodes.values() @@ -222,7 +220,7 @@ impl<'a> IntoIterator for &'a GraphNodes { impl<'a> IntoIterator for &'a mut GraphNodes { type Item = &'a mut GraphNode; - type IntoIter = std::collections::btree_map::ValuesMut<'a, GraphNodeId, GraphNode>; + type IntoIter = indexmap::map::ValuesMut<'a, GraphNodeId, GraphNode>; fn into_iter(self) -> Self::IntoIter { self.nodes.values_mut() From 822ce9c95a717d940228a6a6f0dff7409dcb5108 Mon Sep 17 00:00:00 2001 From: rollrat Date: Sun, 31 May 2026 14:19:17 +0900 Subject: [PATCH 07/11] Remove graph node payload ids Make Graph own node id storage by moving ids out of GraphNode and into GraphNodes entries. Preserve sparse and append-only ids through explicit keyed construction while keeping existing graph transforms and placer code on node references that carry storage ids. --- src/graph/graphviz.rs | 18 +- src/graph/logic.rs | 27 +- src/graph/mod.rs | 444 +++++++++++------- src/graph/module.rs | 8 +- src/graph/world.rs | 26 +- src/sequential/core.rs | 28 +- src/sequential/mod.rs | 19 - src/transform/logic.rs | 110 ++--- .../local_placer/component_tests.rs | 16 +- .../place_and_route/local_placer/mod.rs | 6 +- .../local_placer/sequential/d_latch.rs | 11 +- .../local_placer/sequential/macro_routes.rs | 4 +- .../local_placer/sequential/prefix.rs | 12 +- .../local_placer/sequential/rs_latch.rs | 8 +- .../place_and_route/local_placer/tests.rs | 29 +- src/transform/world.rs | 10 +- src/transform/world_to_logic.rs | 160 ++++--- 17 files changed, 530 insertions(+), 406 deletions(-) diff --git a/src/graph/graphviz.rs b/src/graph/graphviz.rs index 91822a8..d0a161b 100644 --- a/src/graph/graphviz.rs +++ b/src/graph/graphviz.rs @@ -6,7 +6,7 @@ use super::cluster::ClusteredGraph; use super::logic::LogicGraph; use super::module::{GraphModule, GraphWithSubGraphs}; use super::world::WorldGraph; -use super::{Graph, GraphNode, GraphNodeId, SubGraph}; +use super::{Graph, GraphNodeId, GraphNodeRef, SubGraph}; use crate::graph::module::GraphModulePortTarget; pub struct GraphvizBuilder<'a> { @@ -135,7 +135,7 @@ digraph {graph_name} {{ } } - fn print_node(&self, node: &GraphNode) -> String { + fn print_node(&self, node: GraphNodeRef<'_>) -> String { let name = if self.show_node_id { format!("{} #{}", node.kind.name(), node.id) } else { @@ -268,10 +268,14 @@ digraph {graph_name} {{ .nodes .iter() .flat_map(|node| { - node.inputs.iter().map(|input| { - let in_suffix = format!("in{}_{}", input, node.id); - format!(" node{}:out->node{}:{}\n", input, node.id, in_suffix) - }) + let node_id = node.id; + node.inputs + .iter() + .map(move |input| { + let in_suffix = format!("in{}_{}", input, node_id); + format!(" node{}:out->node{}:{}\n", input, node_id, in_suffix) + }) + .collect_vec() }) .collect::>() .join("") @@ -522,7 +526,6 @@ mod tests { #[test] fn table_graphviz_shows_node_tags() { let graph = Graph::from_nodes(vec![GraphNode { - id: 0, kind: GraphNodeKind::None, tag: "Folded RS latch feedback SCC [1, 2, 3, 4]".to_owned(), ..Default::default() @@ -539,7 +542,6 @@ mod tests { #[test] fn graphviz_can_hide_node_tags() { let graph = Graph::from_nodes(vec![GraphNode { - id: 0, kind: GraphNodeKind::None, tag: "debug source tag".to_owned(), ..Default::default() diff --git a/src/graph/logic.rs b/src/graph/logic.rs index 4a70fdb..753ba0f 100644 --- a/src/graph/logic.rs +++ b/src/graph/logic.rs @@ -104,12 +104,14 @@ impl LogicGraph { eyre::bail!("cannot attach output {name}: missing source node {source_id}"); } - self.graph.insert_node(GraphNode { - id: next_id, - kind: GraphNodeKind::Output(name), - inputs: vec![source_id], - ..Default::default() - }); + self.graph.insert_node_with_id( + next_id, + GraphNode { + kind: GraphNodeKind::Output(name), + inputs: vec![source_id], + ..Default::default() + }, + ); next_id += 1; } @@ -139,7 +141,7 @@ impl LogicGraph { .filter_map(|node| match &node.kind { GraphNodeKind::Output(name) => output_source_ids .contains(&node.inputs[0]) - .then_some(name.as_str()), + .then_some(name.clone()), _ => None, }) .collect::>(); @@ -149,6 +151,7 @@ impl LogicGraph { } output_names.sort(); + let output_names = output_names.iter().map(String::as_str).collect_vec(); table.select_outputs(&output_names) } } @@ -311,7 +314,7 @@ pub struct LogicGraphBuilder { stmt: String, node_id: usize, ptr: usize, - nodes: Vec, + nodes: Vec<(GraphNodeId, GraphNode)>, inputs: HashMap, } @@ -346,7 +349,7 @@ impl LogicGraphBuilder { pub fn build(mut self, output_name: String) -> eyre::Result { self.do_parse(output_name); - let mut graph = Graph::from_nodes(self.nodes.clone()); + let mut graph = Graph::from_nodes_with_ids(self.nodes.clone()); graph.build_outputs(); Ok(LogicGraph { graph }) @@ -365,14 +368,14 @@ impl LogicGraphBuilder { } fn new_node(&mut self, kind: GraphNodeKind, inputs: Vec) -> GraphNodeId { + let id = self.next_id(); let node = GraphNode { - id: self.next_id(), kind, inputs, ..Default::default() }; - self.nodes.push(node); - self.nodes.last().unwrap().clone().id + self.nodes.push((id, node)); + id } fn new_input_node(&mut self, name: String) -> GraphNodeId { diff --git a/src/graph/mod.rs b/src/graph/mod.rs index 2c11cf2..5a6340f 100644 --- a/src/graph/mod.rs +++ b/src/graph/mod.rs @@ -116,7 +116,6 @@ impl GraphNodeKind { #[derive(Default, Debug, Clone)] pub struct GraphNode { - pub id: GraphNodeId, pub kind: GraphNodeKind, pub inputs: Vec, pub outputs: Vec, @@ -127,8 +126,7 @@ impl Display for GraphNode { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!( f, - "N{} {:?} := {} {:?} {}", - self.id, + "{:?} := {} {:?} {}", self.outputs, self.kind.name(), self.inputs, @@ -138,15 +136,83 @@ impl Display for GraphNode { } impl GraphNode { - pub fn new(id: GraphNodeId, kind: GraphNodeKind) -> Self { + pub fn new(kind: GraphNodeKind) -> Self { Self { - id, kind, ..Default::default() } } } +#[derive(Clone, Copy)] +pub struct GraphNodeRef<'a> { + pub id: GraphNodeId, + node: &'a GraphNode, +} + +impl GraphNodeRef<'_> { + pub fn new(id: GraphNodeId, node: &GraphNode) -> GraphNodeRef<'_> { + GraphNodeRef { id, node } + } + + pub fn clone_node(&self) -> GraphNode { + self.node.clone() + } +} + +impl std::ops::Deref for GraphNodeRef<'_> { + type Target = GraphNode; + + fn deref(&self) -> &Self::Target { + self.node + } +} + +impl Display for GraphNodeRef<'_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + Display::fmt(self.node, f) + } +} + +pub struct GraphNodeMut<'a> { + pub id: GraphNodeId, + node: &'a mut GraphNode, +} + +impl std::ops::Deref for GraphNodeMut<'_> { + type Target = GraphNode; + + fn deref(&self) -> &Self::Target { + self.node + } +} + +impl std::ops::DerefMut for GraphNodeMut<'_> { + fn deref_mut(&mut self) -> &mut Self::Target { + self.node + } +} + +#[derive(Debug, Clone)] +pub struct GraphNodeEntry { + pub id: GraphNodeId, + pub node: GraphNode, +} + +impl std::ops::Deref for GraphNodeEntry { + type Target = GraphNode; + + fn deref(&self) -> &Self::Target { + &self.node + } +} + +impl std::ops::DerefMut for GraphNodeEntry { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.node + } +} + #[derive(Default, Debug, Clone)] pub struct GraphNodes { nodes: IndexMap, @@ -161,12 +227,26 @@ impl GraphNodes { self.nodes.is_empty() } - pub fn iter(&self) -> indexmap::map::Values<'_, GraphNodeId, GraphNode> { - self.nodes.values() + pub fn get(&self, id: GraphNodeId) -> Option> { + self.nodes.get(&id).map(|node| GraphNodeRef { id, node }) } - pub fn iter_mut(&mut self) -> indexmap::map::ValuesMut<'_, GraphNodeId, GraphNode> { - self.nodes.values_mut() + pub fn get_mut(&mut self, id: GraphNodeId) -> Option> { + self.nodes + .get_mut(&id) + .map(|node| GraphNodeMut { id, node }) + } + + pub fn iter(&self) -> impl Iterator> { + self.nodes + .iter() + .map(|(&id, node)| GraphNodeRef { id, node }) + } + + pub fn iter_mut(&mut self) -> impl Iterator> { + self.nodes + .iter_mut() + .map(|(&id, node)| GraphNodeMut { id, node }) } pub fn retain(&mut self, f: impl FnMut(&GraphNode) -> bool) { @@ -175,55 +255,70 @@ impl GraphNodes { } pub fn remove(&mut self, index: usize) -> GraphNode { + self.remove_entry(index).node + } + + pub fn remove_entry(&mut self, index: usize) -> GraphNodeEntry { self.nodes .shift_remove_index(index) - .map(|(_, node)| node) + .map(|(id, node)| GraphNodeEntry { id, node }) .expect("node index must exist") } - pub fn extend(&mut self, nodes: impl IntoIterator) { - nodes.into_iter().for_each(|node| self.insert(node)); + pub fn shift_id(&mut self, from: GraphNodeId, to: GraphNodeId) { + if let Some(node) = self.nodes.shift_remove(&from) { + self.nodes.insert(to, node); + } + } + + pub fn extend(&mut self, nodes: impl IntoIterator) { + nodes + .into_iter() + .for_each(|entry| self.insert_with_id(entry.id, entry.node)); } - pub fn insert(&mut self, node: GraphNode) { - self.nodes.insert(node.id, node); - self.nodes.sort_by(|id, _, other_id, _| id.cmp(other_id)); + pub fn insert_with_id(&mut self, id: GraphNodeId, node: GraphNode) { + self.nodes.insert(id, node); } } -impl From> for GraphNodes { - fn from(mut nodes: Vec) -> Self { - nodes.sort_by_key(|node| node.id); +impl From> for GraphNodes { + fn from(mut nodes: Vec<(GraphNodeId, GraphNode)>) -> Self { + nodes.sort_by_key(|(id, _)| *id); Self { - nodes: nodes.into_iter().map(|node| (node.id, node)).collect(), + nodes: nodes.into_iter().collect(), } } } impl IntoIterator for GraphNodes { - type Item = GraphNode; - type IntoIter = indexmap::map::IntoValues; + type Item = GraphNodeEntry; + type IntoIter = std::vec::IntoIter; fn into_iter(self) -> Self::IntoIter { - self.nodes.into_values() + self.nodes + .into_iter() + .map(|(id, node)| GraphNodeEntry { id, node }) + .collect_vec() + .into_iter() } } impl<'a> IntoIterator for &'a GraphNodes { - type Item = &'a GraphNode; - type IntoIter = indexmap::map::Values<'a, GraphNodeId, GraphNode>; + type Item = GraphNodeRef<'a>; + type IntoIter = std::vec::IntoIter>; fn into_iter(self) -> Self::IntoIter { - self.nodes.values() + self.iter().collect_vec().into_iter() } } impl<'a> IntoIterator for &'a mut GraphNodes { - type Item = &'a mut GraphNode; - type IntoIter = indexmap::map::ValuesMut<'a, GraphNodeId, GraphNode>; + type Item = GraphNodeMut<'a>; + type IntoIter = std::vec::IntoIter>; fn into_iter(self) -> Self::IntoIter { - self.nodes.values_mut() + self.iter_mut().collect_vec().into_iter() } } @@ -236,12 +331,8 @@ pub struct Graph { } impl Graph { - pub fn from_nodes(nodes: Vec) -> Self { - let next_node_id = nodes - .iter() - .map(|node| node.id) - .max() - .map_or(0, |id| id + 1); + pub fn from_nodes_with_ids(nodes: Vec<(GraphNodeId, GraphNode)>) -> Self { + let next_node_id = nodes.iter().map(|(id, _)| *id).max().map_or(0, |id| id + 1); Self { nodes: nodes.into(), next_node_id, @@ -249,6 +340,11 @@ impl Graph { } } + pub fn from_nodes(nodes: Vec) -> Self { + let nodes = nodes.into_iter().enumerate().collect_vec(); + Self::from_nodes_with_ids(nodes) + } + fn to_petgraph_with_node_ids(&self) -> petgraph::Graph { let mut graph = petgraph::Graph::::new(); let mut id_to_index = HashMap::new(); @@ -339,17 +435,16 @@ impl Graph { let mut edges = nodes .iter() .flat_map(|node_id| { - let node = self.find_node_by_id(*node_id).into_iter(); - node.flat_map(move |node| { - if incoming { - node.inputs.iter() - } else { - node.outputs.iter() - } - }) + self.find_node_by_id(*node_id) + .map_or_else(Vec::new, |node| { + if incoming { + node.inputs.clone() + } else { + node.outputs.clone() + } + }) }) .filter(|node_id| !nodes.contains(node_id)) - .copied() .collect::>(); edges.sort(); edges.dedup(); @@ -401,7 +496,7 @@ impl Graph { self.consumers.extend(other.consumers); } - // src와 other의 input이 같은 것 끼리 연결하여 merge함 + // src?€ other??input??媛숈? 寃??쇰━ ?곌껐?섏뿬 merge?? pub fn merge_by_input(&mut self, mut other: Self) { if let Some(id) = self.max_node_id() { other.rebuild_node_id_base(id + 1); @@ -428,13 +523,13 @@ impl Graph { .collect(); for (from, to) in replace_targets { - let node = other.nodes.iter_mut().find(|node| node.id == from).unwrap(); + let outputs = other.find_node_by_id(from).unwrap().outputs.clone(); self.find_node_by_id_mut(to) .unwrap() .outputs - .extend(node.outputs.clone()); - node.id = to; + .extend(outputs); + other.replace_node_id_lazy(from, to); } other.build_producers(); @@ -448,7 +543,7 @@ impl Graph { self.build_consumers(); } - // src의 output을 target의 input과 연결하여 merge함 + // src??output??target??input怨??곌껐?섏뿬 merge?? pub fn merge_by_outin(&mut self, mut target: Self, out_in: Vec<(&str, &str)>) { if let Some(id) = self.max_node_id() { target.rebuild_node_id_base(id + 1); @@ -503,7 +598,7 @@ impl Graph { self.build_consumers(); } - // self의 input, output들을 target의 input과 연결함 + // self??input, output?ㅼ쓣 target??input怨??곌껐?? pub fn merge(&mut self, mut target: Self) { if let Some(id) = self.max_node_id() { target.rebuild_node_id_base(id + 1); @@ -513,7 +608,7 @@ impl Graph { .nodes .iter() .filter_map(|node| match &node.kind { - GraphNodeKind::Input(name) | GraphNodeKind::Output(name) => Some(name.as_str()), + GraphNodeKind::Input(name) | GraphNodeKind::Output(name) => Some(name.to_owned()), _ => None, }) .collect(); @@ -525,7 +620,7 @@ impl Graph { GraphNodeKind::Input(name) | GraphNodeKind::Output(name) => Some(name.to_owned()), _ => None, }) - .filter(|name| target_inputs.contains(name.as_str())) + .filter(|name| target_inputs.contains(name)) .collect(); let src_inputs: HashMap = self @@ -593,7 +688,7 @@ impl Graph { } pub fn replace_input_name(&mut self, from: &str, to: String) -> bool { - if let Some(node) = self + if let Some(mut node) = self .nodes .iter_mut() .find(|node| matches!(&node.kind, GraphNodeKind::Input(name) if name == from)) @@ -606,7 +701,7 @@ impl Graph { } pub fn replace_output_name(&mut self, from: &str, to: String) -> bool { - if let Some(node) = self + if let Some(mut node) = self .nodes .iter_mut() .find(|node| matches!(&node.kind, GraphNodeKind::Output(name) if name == from)) @@ -640,36 +735,35 @@ impl Graph { id } - pub fn add_node(&mut self, mut node: GraphNode) -> GraphNodeId { + pub fn add_node(&mut self, node: GraphNode) -> GraphNodeId { let id = self.allocate_node_id(); - node.id = id; - self.insert_node(node); + self.insert_node_with_id(id, node); id } - pub fn insert_node(&mut self, node: GraphNode) { - self.next_node_id = self.next_node_id.max(node.id + 1); - self.nodes.insert(node); + pub(crate) fn insert_node_with_id(&mut self, id: GraphNodeId, node: GraphNode) { + self.next_node_id = self.next_node_id.max(id + 1); + self.nodes.insert_with_id(id, node); } - pub fn find_node_by_input_name(&mut self, input_name: &str) -> Option<&mut GraphNode> { + pub fn find_node_by_input_name(&mut self, input_name: &str) -> Option> { self.nodes .iter_mut() .find(|node| matches!(&node.kind, GraphNodeKind::Input(name) if name == input_name)) } - pub fn find_node_by_output_name(&mut self, output_name: &str) -> Option<&mut GraphNode> { + pub fn find_node_by_output_name(&mut self, output_name: &str) -> Option> { self.nodes .iter_mut() .find(|node| matches!(&node.kind, GraphNodeKind::Output(name) if name == output_name)) } - pub fn find_node_by_id(&self, node_id: GraphNodeId) -> Option<&GraphNode> { - self.nodes.iter().find(|node| node.id == node_id) + pub fn find_node_by_id(&self, node_id: GraphNodeId) -> Option> { + self.nodes.get(node_id) } - pub fn find_node_by_id_mut(&mut self, node_id: GraphNodeId) -> Option<&mut GraphNode> { - self.nodes.iter_mut().find(|node| node.id == node_id) + pub fn find_node_by_id_mut(&mut self, node_id: GraphNodeId) -> Option> { + self.nodes.get_mut(node_id) } pub fn find_and_remove_node_by_id(&mut self, node_id: GraphNodeId) -> Option { @@ -678,12 +772,17 @@ impl Graph { } pub fn rebuild_node_id_base(&mut self, base_index: usize) { - for node in &mut self.nodes { - node.id += base_index; - for index in itertools::chain!(&mut node.inputs, &mut node.outputs) { - *index += base_index; - } - } + let nodes = std::mem::take(&mut self.nodes) + .into_iter() + .map(|entry| { + let mut node = entry.node; + for index in itertools::chain!(&mut node.inputs, &mut node.outputs) { + *index += base_index; + } + (entry.id + base_index, node) + }) + .collect_vec(); + self.nodes = GraphNodes::from(nodes); self.next_node_id = self.next_node_id(); self.build_producers(); @@ -709,7 +808,7 @@ impl Graph { let replacement_id = self.next_node_id(); for input in &inputs { - if let Some(node) = self.find_node_by_id_mut(*input) { + if let Some(mut node) = self.find_node_by_id_mut(*input) { node.outputs.retain(|id| !removed_nodes.contains(id)); if !node.outputs.contains(&replacement_id) { node.outputs.push(replacement_id); @@ -717,7 +816,7 @@ impl Graph { } } for output in &outputs { - if let Some(node) = self.find_node_by_id_mut(*output) { + if let Some(mut node) = self.find_node_by_id_mut(*output) { node.inputs.retain(|id| !removed_nodes.contains(id)); if !node.inputs.contains(&replacement_id) { node.inputs.push(replacement_id); @@ -728,13 +827,15 @@ impl Graph { self.remove_by_node_id_lazy(*node_id); } - self.insert_node(GraphNode { - id: replacement_id, - kind, - inputs, - outputs, - tag, - }); + self.insert_node_with_id( + replacement_id, + GraphNode { + kind, + inputs, + outputs, + tag, + }, + ); self.build_inputs(); self.build_outputs(); self.build_producers(); @@ -743,7 +844,7 @@ impl Graph { replacement_id } - // 노드 삭제하고 삭제한 노드의 Input Output끼리 연결함 + // ?몃뱶 ??젣?섍퀬 ??젣???몃뱶??Input Output?쇰━ ?곌껐?? pub fn remove_and_reconnect_by_node_id_lazy(&mut self, node_id: GraphNodeId) { let Some(index) = self.nodes.iter().position(|node| node.id == node_id) else { return; @@ -752,40 +853,51 @@ impl Graph { let node = self.nodes.remove(index); for input in &node.inputs { - if let Some(input) = self.find_node_by_id_mut(*input) { + if let Some(mut input) = self.find_node_by_id_mut(*input) { input.outputs.extend(&node.outputs); } } for output in &node.outputs { - if let Some(output) = self.find_node_by_id_mut(*output) { + if let Some(mut output) = self.find_node_by_id_mut(*output) { output.inputs.extend(&node.inputs); } } } pub fn replace_node_id_lazy(&mut self, from: GraphNodeId, to: GraphNodeId) { - self.nodes - .iter_mut() - .flat_map(|node| itertools::chain!(&mut node.inputs, &mut node.outputs)) - .filter(|node_id| **node_id == from) - .for_each(|node_id| *node_id = to); + for mut node in self.nodes.iter_mut() { + for node_id in &mut node.inputs { + if *node_id == from { + *node_id = to; + } + } + for node_id in &mut node.outputs { + if *node_id == from { + *node_id = to; + } + } + } } pub fn replace_input_node_id_lazy(&mut self, from: GraphNodeId, to: GraphNodeId) { - self.nodes - .iter_mut() - .flat_map(|node| &mut node.inputs) - .filter(|node_id| **node_id == from) - .for_each(|node_id| *node_id = to); + for mut node in self.nodes.iter_mut() { + for node_id in &mut node.inputs { + if *node_id == from { + *node_id = to; + } + } + } } pub fn replace_output_node_id_lazy(&mut self, from: GraphNodeId, to: GraphNodeId) { - self.nodes - .iter_mut() - .flat_map(|node| &mut node.outputs) - .filter(|node_id| **node_id == from) - .for_each(|node_id| *node_id = to); + for mut node in self.nodes.iter_mut() { + for node_id in &mut node.outputs { + if *node_id == from { + *node_id = to; + } + } + } } pub fn replace_target_input_node_ids( @@ -794,7 +906,7 @@ impl Graph { from: GraphNodeId, to: Vec, ) { - let node = self.find_node_by_id_mut(target).unwrap(); + let mut node = self.find_node_by_id_mut(target).unwrap(); node.inputs.retain(|id| *id != from); node.inputs.extend(to); } @@ -805,12 +917,12 @@ impl Graph { from: GraphNodeId, to: Vec, ) { - let node = self.find_node_by_id_mut(target).unwrap(); + let mut node = self.find_node_by_id_mut(target).unwrap(); node.outputs.retain(|id| *id != from); node.outputs.extend(to); } - // outputs이 반드시 determine 되어야함 + // outputs??諛섎뱶??determine ?섏뼱?쇳븿 pub fn build_inputs(&mut self) { let mut input_map: HashMap> = HashMap::new(); @@ -823,7 +935,7 @@ impl Graph { }); }); - for node in self.nodes.iter_mut() { + for mut node in self.nodes.iter_mut() { node.inputs = input_map .get(&node.id) .map(|ids| ids.iter().copied().sorted().collect_vec()) @@ -831,7 +943,7 @@ impl Graph { } } - // inputs이 반드시 determine 되어야함 + // inputs??諛섎뱶??determine ?섏뼱?쇳븿 pub fn build_outputs(&mut self) { let mut output_map: HashMap> = HashMap::new(); @@ -844,7 +956,7 @@ impl Graph { }); }); - for node in self.nodes.iter_mut() { + for mut node in self.nodes.iter_mut() { node.outputs = output_map .get(&node.id) .map(|ids| ids.iter().copied().sorted().collect()) @@ -904,14 +1016,14 @@ impl Graph { .nodes .iter() .filter(|node| node_ids.contains(&node.id)) - .cloned() + .map(|node| (node.id, node.node.clone())) .collect_vec(); - for node in &mut nodes { + for (_, node) in &mut nodes { node.inputs.retain(|input| node_ids.contains(input)); node.outputs.retain(|output| node_ids.contains(output)); } - let mut graph = Self::from_nodes(nodes); + let mut graph = Self::from_nodes_with_ids(nodes); graph.build_inputs(); graph.build_outputs(); graph.build_producers(); @@ -995,7 +1107,7 @@ impl Graph { |node| matches!(&node.kind, GraphNodeKind::Output(name) if name == output_name), )?; - let id = self.nodes.remove(index).id; + let id = self.nodes.remove_entry(index).id; self.build_outputs(); self.build_producers(); self.build_consumers(); @@ -1116,15 +1228,15 @@ impl From<&SubGraphWithGraph<'_>> for Graph { .nodes .iter() .filter(|node| node_ids.contains(&node.id)) - .cloned() + .map(|node| (node.id, node.node.clone())) .collect_vec(); - for node in &mut nodes { + for (_, node) in &mut nodes { node.inputs.retain(|input| node_ids.contains(input)); node.outputs.retain(|output| node_ids.contains(output)); } - let mut graph = Graph::from_nodes(nodes); + let mut graph = Graph::from_nodes_with_ids(nodes); graph.rebuild_node_id_base(0); graph @@ -1163,15 +1275,17 @@ pub fn subgraphs_to_clustered_graph(graph: &Graph, subgraphs: &[SubGraph]) -> Cl clustered_type: ClusteredType::Cluster(sg.nodes.clone()), }; - GraphNode { - id: index, - kind: GraphNodeKind::Clustered(clustered), - ..Default::default() - } + ( + index, + GraphNode { + kind: GraphNodeKind::Clustered(clustered), + ..Default::default() + }, + ) }) .collect_vec(); - let mut weighted_node = subgraphs + let weighted_node = subgraphs .iter() .enumerate() .map(|(index, sg)| { @@ -1209,18 +1323,28 @@ pub fn subgraphs_to_clustered_graph(graph: &Graph, subgraphs: &[SubGraph]) -> Cl }) .collect_vec(); - // Put id lazy - for (index, node) in weighted_node.iter_mut().flatten().enumerate() { - node.id = nodes.len() + index + 1; + let weighted_base_id = nodes.len(); + let mut weighted_nodes = Vec::new(); + let mut weighted_index = 0; + let mut weighted_outputs = Vec::new(); + for nodes in weighted_node { + let mut outputs = Vec::new(); + for node in nodes { + let id = weighted_base_id + weighted_index + 1; + outputs.push(id); + weighted_nodes.push((id, node)); + weighted_index += 1; + } + weighted_outputs.push(outputs); } // Set output weighted node - for (index, clustered_node) in nodes.iter_mut().enumerate() { - clustered_node.outputs = weighted_node[index].iter().map(|node| node.id).collect(); + for (index, (_, clustered_node)) in nodes.iter_mut().enumerate() { + clustered_node.outputs = weighted_outputs[index].clone(); } - let mut graph = - Graph::from_nodes([nodes, weighted_node.into_iter().flatten().collect_vec()].concat()); + nodes.extend(weighted_nodes); + let mut graph = Graph::from_nodes_with_ids(nodes); graph.build_inputs(); graph.build_producers(); @@ -1239,22 +1363,18 @@ mod tests { fn build_inputs_and_outputs_are_sorted() { let mut graph = Graph::from_nodes(vec![ GraphNode { - id: 0, outputs: vec![3], ..Default::default() }, GraphNode { - id: 1, outputs: vec![3], ..Default::default() }, GraphNode { - id: 2, outputs: vec![3], ..Default::default() }, GraphNode { - id: 3, inputs: vec![2, 0, 1], ..Default::default() }, @@ -1273,28 +1393,23 @@ mod tests { fn extract_graph_by_node_ids_keeps_only_internal_edges() { let mut graph = Graph::from_nodes(vec![ GraphNode { - id: 0, outputs: vec![2], ..Default::default() }, GraphNode { - id: 1, outputs: vec![2], ..Default::default() }, GraphNode { - id: 2, inputs: vec![0, 1], outputs: vec![3, 4], ..Default::default() }, GraphNode { - id: 3, inputs: vec![2], ..Default::default() }, GraphNode { - id: 4, inputs: vec![2], ..Default::default() }, @@ -1332,19 +1447,16 @@ mod tests { fn sequential_node_kind_is_named_and_verified() -> eyre::Result<()> { let mut graph = Graph::from_nodes(vec![ GraphNode { - id: 0, kind: GraphNodeKind::Input("s".to_owned()), outputs: vec![2], ..Default::default() }, GraphNode { - id: 1, kind: GraphNodeKind::Input("r".to_owned()), outputs: vec![2], ..Default::default() }, GraphNode { - id: 2, kind: GraphNodeKind::Sequential(SequentialPrimitive::new( SequentialType::RsLatch, vec!["s".to_owned(), "r".to_owned()], @@ -1355,7 +1467,6 @@ mod tests { ..Default::default() }, GraphNode { - id: 3, kind: GraphNodeKind::Output("q".to_owned()), inputs: vec![2], ..Default::default() @@ -1379,23 +1490,29 @@ mod tests { #[test] fn sparse_node_ids_are_keyed_and_append_only() { - let mut graph = Graph::from_nodes(vec![ - GraphNode { - id: 30, - inputs: vec![20], - ..Default::default() - }, - GraphNode { - id: 10, - outputs: vec![20], - ..Default::default() - }, - GraphNode { - id: 20, - inputs: vec![10], - outputs: vec![30], - ..Default::default() - }, + let mut graph = Graph::from_nodes_with_ids(vec![ + ( + 30, + GraphNode { + inputs: vec![20], + ..Default::default() + }, + ), + ( + 10, + GraphNode { + outputs: vec![20], + ..Default::default() + }, + ), + ( + 20, + GraphNode { + inputs: vec![10], + outputs: vec![30], + ..Default::default() + }, + ), ]); assert_eq!( @@ -1412,17 +1529,22 @@ mod tests { assert_eq!(petgraph.node_count(), 3); assert_eq!(petgraph.edge_count(), 2); - graph.insert_node(GraphNode { - id: 25, - tag: "replacement".to_owned(), - ..Default::default() - }); + graph.insert_node_with_id( + 25, + GraphNode { + tag: "replacement".to_owned(), + ..Default::default() + }, + ); assert_eq!( graph.nodes.iter().map(|node| node.id).collect_vec(), - vec![10, 20, 25, 30] + vec![10, 20, 30, 25] ); assert_eq!(graph.find_node_by_id(25).unwrap().tag, "replacement"); - assert_eq!(graph.find_and_remove_node_by_id(25).unwrap().id, 25); + assert_eq!( + graph.find_and_remove_node_by_id(25).unwrap().tag, + "replacement" + ); graph.remove_by_node_id_lazy(30); let id = graph.add_node(GraphNode::default()); diff --git a/src/graph/module.rs b/src/graph/module.rs index 46a2c8a..747d71e 100644 --- a/src/graph/module.rs +++ b/src/graph/module.rs @@ -131,7 +131,8 @@ impl From for GraphModule { .inputs() .iter() .map(|input| { - let input_name = value.find_node_by_id(*input).unwrap().kind.as_input(); + let input_node = value.find_node_by_id(*input).unwrap(); + let input_name = input_node.kind.as_input(); GraphModulePort { port_type: GraphModulePortType::InputNet, target: GraphModulePortTarget::Node(input_name.clone()), @@ -143,7 +144,8 @@ impl From for GraphModule { .outputs() .iter() .map(|output| { - let output_name = value.find_node_by_id(*output).unwrap().kind.as_output(); + let output_node = value.find_node_by_id(*output).unwrap(); + let output_name = output_node.kind.as_output(); GraphModulePort { port_type: GraphModulePortType::OutputNet, target: GraphModulePortTarget::Node(output_name.clone()), @@ -244,7 +246,7 @@ impl From<(&GraphModuleContext, &GraphModule)> for GraphWithSubGraphs { graph .nodes .iter_mut() - .for_each(|node| match &mut node.kind { + .for_each(|mut node| match &mut node.kind { GraphNodeKind::Input(name) | GraphNodeKind::Output(name) => { if let Some(port_name) = module_port_name .get_mut(instance) diff --git a/src/graph/world.rs b/src/graph/world.rs index 5face67..039be71 100644 --- a/src/graph/world.rs +++ b/src/graph/world.rs @@ -97,7 +97,7 @@ impl WorldGraphBuilder { self.visit_blocks(); let (nodes, positions) = self.build_nodes(); - let mut graph = Graph::from_nodes(nodes); + let mut graph = Graph::from_nodes_with_ids(nodes); graph.build_inputs(); graph.build_producers(); @@ -143,7 +143,12 @@ impl WorldGraphBuilder { } } - fn build_nodes(&mut self) -> (Vec, HashMap) { + fn build_nodes( + &mut self, + ) -> ( + Vec<(GraphNodeId, GraphNode)>, + HashMap, + ) { let mut graph_id: HashMap = HashMap::new(); let mut nodes: HashMap = HashMap::new(); @@ -154,10 +159,7 @@ impl WorldGraphBuilder { .enumerate() { graph_id.insert(*pos, index); - nodes.insert( - *pos, - GraphNode::new(index, GraphNodeKind::Block(self.world[*pos])), - ); + nodes.insert(*pos, GraphNode::new(GraphNodeKind::Block(self.world[*pos]))); } for pos in self @@ -194,10 +196,16 @@ impl WorldGraphBuilder { } } - let positions = nodes.iter().map(|(pos, node)| (node.id, *pos)).collect(); - let mut nodes = nodes.into_values().collect_vec(); + let positions = nodes + .keys() + .map(|pos| (graph_id[pos], *pos)) + .collect::>(); + let mut nodes = nodes + .into_iter() + .map(|(pos, node)| (graph_id[&pos], node)) + .collect_vec(); - nodes.sort_by(|a, b| a.id.cmp(&b.id)); + nodes.sort_by_key(|(id, _)| *id); (nodes, positions) } diff --git a/src/sequential/core.rs b/src/sequential/core.rs index d72e981..cd3ad16 100644 --- a/src/sequential/core.rs +++ b/src/sequential/core.rs @@ -105,25 +105,29 @@ pub fn rs_latch_prefix_graph(graph: &Graph, core: &RsLatchCore) -> Option>(); let mut next_node_id = graph.next_node_id(); for (output_name, source_node_id) in output_roots { - nodes.push(GraphNode { - id: next_node_id, - kind: GraphNodeKind::Output(output_name.to_owned()), - inputs: vec![source_node_id], - ..Default::default() - }); + nodes.push(( + next_node_id, + GraphNode { + kind: GraphNodeKind::Output(output_name.to_owned()), + inputs: vec![source_node_id], + ..Default::default() + }, + )); next_node_id += 1; } - let mut graph = Graph::from_nodes(nodes); + let mut graph = Graph::from_nodes_with_ids(nodes); graph.build_outputs(); graph.build_producers(); graph.build_consumers(); diff --git a/src/sequential/mod.rs b/src/sequential/mod.rs index 427be0b..6305932 100644 --- a/src/sequential/mod.rs +++ b/src/sequential/mod.rs @@ -105,17 +105,14 @@ fn rs_latch_inner_graph() -> Graph { // q = ~(r | nq), nq = ~(s | q) let mut graph = Graph::from_nodes(vec![ GraphNode { - id: 0, kind: GraphNodeKind::Input("s".to_owned()), ..Default::default() }, GraphNode { - id: 1, kind: GraphNodeKind::Input("r".to_owned()), ..Default::default() }, GraphNode { - id: 2, kind: GraphNodeKind::Logic(Logic { logic_type: LogicType::Or, }), @@ -123,7 +120,6 @@ fn rs_latch_inner_graph() -> Graph { ..Default::default() }, GraphNode { - id: 3, kind: GraphNodeKind::Logic(Logic { logic_type: LogicType::Not, }), @@ -131,7 +127,6 @@ fn rs_latch_inner_graph() -> Graph { ..Default::default() }, GraphNode { - id: 4, kind: GraphNodeKind::Logic(Logic { logic_type: LogicType::Or, }), @@ -139,7 +134,6 @@ fn rs_latch_inner_graph() -> Graph { ..Default::default() }, GraphNode { - id: 5, kind: GraphNodeKind::Logic(Logic { logic_type: LogicType::Not, }), @@ -147,13 +141,11 @@ fn rs_latch_inner_graph() -> Graph { ..Default::default() }, GraphNode { - id: 6, kind: GraphNodeKind::Output("q".to_owned()), inputs: vec![3], ..Default::default() }, GraphNode { - id: 7, kind: GraphNodeKind::Output("nq".to_owned()), inputs: vec![5], ..Default::default() @@ -170,17 +162,14 @@ fn d_latch_inner_graph() -> Graph { // s = d & en, r = ~d & en, q = ~(r | nq), nq = ~(s | q) let mut graph = Graph::from_nodes(vec![ GraphNode { - id: 0, kind: GraphNodeKind::Input("d".to_owned()), ..Default::default() }, GraphNode { - id: 1, kind: GraphNodeKind::Input("en".to_owned()), ..Default::default() }, GraphNode { - id: 2, kind: GraphNodeKind::Logic(Logic { logic_type: LogicType::And, }), @@ -188,7 +177,6 @@ fn d_latch_inner_graph() -> Graph { ..Default::default() }, GraphNode { - id: 3, kind: GraphNodeKind::Logic(Logic { logic_type: LogicType::Not, }), @@ -196,7 +184,6 @@ fn d_latch_inner_graph() -> Graph { ..Default::default() }, GraphNode { - id: 4, kind: GraphNodeKind::Logic(Logic { logic_type: LogicType::And, }), @@ -204,7 +191,6 @@ fn d_latch_inner_graph() -> Graph { ..Default::default() }, GraphNode { - id: 5, kind: GraphNodeKind::Logic(Logic { logic_type: LogicType::Or, }), @@ -212,7 +198,6 @@ fn d_latch_inner_graph() -> Graph { ..Default::default() }, GraphNode { - id: 6, kind: GraphNodeKind::Logic(Logic { logic_type: LogicType::Not, }), @@ -220,7 +205,6 @@ fn d_latch_inner_graph() -> Graph { ..Default::default() }, GraphNode { - id: 7, kind: GraphNodeKind::Logic(Logic { logic_type: LogicType::Or, }), @@ -228,7 +212,6 @@ fn d_latch_inner_graph() -> Graph { ..Default::default() }, GraphNode { - id: 8, kind: GraphNodeKind::Logic(Logic { logic_type: LogicType::Not, }), @@ -236,13 +219,11 @@ fn d_latch_inner_graph() -> Graph { ..Default::default() }, GraphNode { - id: 9, kind: GraphNodeKind::Output("q".to_owned()), inputs: vec![6], ..Default::default() }, GraphNode { - id: 10, kind: GraphNodeKind::Output("nq".to_owned()), inputs: vec![8], ..Default::default() diff --git a/src/transform/logic.rs b/src/transform/logic.rs index b12ae6e..be63a66 100644 --- a/src/transform/logic.rs +++ b/src/transform/logic.rs @@ -226,7 +226,7 @@ impl LogicGraphTransformer { let tar_output_input = tar.find_node_by_id(tar.outputs()[0]).unwrap().inputs[0]; for index in 0..=1 { - if let Some(node) = g.find_node_by_id_mut(src_inputs[index]) { + if let Some(mut node) = g.find_node_by_id_mut(src_inputs[index]) { node.outputs.extend(tar_inputs_outputs[index].clone()); node.outputs.retain(|node_id| *node_id != src); } @@ -260,12 +260,12 @@ impl LogicGraphTransformer { .nodes .iter() .filter(|node| matches!(node.kind, GraphNodeKind::Input(_))) - .cloned() + .map(|node| (node.id, node.clone_node())) .collect_vec(); let input_terms = input_nodes .iter() .enumerate() - .map(|(index, node)| (node.id, index as u8)) + .map(|(index, (node_id, _))| (*node_id, index as u8)) .collect::>(); fn make_qmc_form( @@ -311,9 +311,9 @@ impl LogicGraphTransformer { let mut nodes = Vec::new(); fn make_rc_form( - nodes: &mut Vec, + nodes: &mut Vec<(GraphNodeId, GraphNode)>, id: &mut usize, - lookup: &Vec, + lookup: &Vec<(GraphNodeId, GraphNode)>, node: &quine_mc_cluskey::Bool, ) -> GraphNodeId { let tid = *id; @@ -321,12 +321,10 @@ impl LogicGraphTransformer { let node = match node { quine_mc_cluskey::Bool::Term(v) => GraphNode { - id: tid, - kind: GraphNodeKind::Input(lookup[*v as usize].kind.as_input().to_owned()), + kind: GraphNodeKind::Input(lookup[*v as usize].1.kind.as_input().to_owned()), ..Default::default() }, quine_mc_cluskey::Bool::And(op) => GraphNode { - id: tid, kind: GraphNodeKind::Logic(Logic { logic_type: LogicType::And, }), @@ -337,7 +335,6 @@ impl LogicGraphTransformer { ..Default::default() }, quine_mc_cluskey::Bool::Or(op) => GraphNode { - id: tid, kind: GraphNodeKind::Logic(Logic { logic_type: LogicType::Or, }), @@ -348,7 +345,6 @@ impl LogicGraphTransformer { ..Default::default() }, quine_mc_cluskey::Bool::Not(v) => GraphNode { - id: tid, kind: GraphNodeKind::Logic(Logic { logic_type: LogicType::Not, }), @@ -359,23 +355,22 @@ impl LogicGraphTransformer { }; if let GraphNodeKind::Input(name) = &node.kind { - if let Some(node) = nodes - .iter() - .find(|node| matches!(&node.kind, GraphNodeKind::Input(other) if name == other)) - { + if let Some((node_id, _)) = nodes.iter().find( + |(_, node)| matches!(&node.kind, GraphNodeKind::Input(other) if name == other), + ) { *id -= 1; - return node.id; + return *node_id; } } - nodes.push(node); + nodes.push((tid, node)); tid } let mut id = 0; let node = make_rc_form(&mut nodes, &mut id, &input_nodes, &results[0]); + let output_id = nodes.len(); let output_node = GraphNode { - id: nodes.len(), kind: GraphNodeKind::Output( self.graph .graph @@ -388,8 +383,8 @@ impl LogicGraphTransformer { inputs: vec![node], ..Default::default() }; - nodes.push(output_node); - self.graph.graph = Graph::from_nodes(nodes); + nodes.push((output_id, output_node)); + self.graph.graph = Graph::from_nodes_with_ids(nodes); self.graph.graph.build_outputs(); self.graph.graph.build_producers(); self.graph.graph.build_consumers(); @@ -469,22 +464,22 @@ impl LogicGraphTransformer { } pub fn insert_buffers_for_direct_or_to_or(&mut self) -> eyre::Result<()> { - let direct_edges = self - .graph - .graph - .nodes - .iter() - .filter(|node| { - matches!(&node.kind, GraphNodeKind::Logic(logic) if logic.logic_type == LogicType::Or) - }) - .flat_map(|node| { - node.outputs.iter().filter_map(|&output_id| { - let output = self.graph.graph.find_node_by_id(output_id)?; - matches!(&output.kind, GraphNodeKind::Logic(logic) if logic.logic_type == LogicType::Or) - .then_some((node.id, output.id)) - }) - }) - .collect_vec(); + let mut direct_edges = Vec::new(); + for node in self.graph.graph.nodes.iter() { + if !matches!(&node.kind, GraphNodeKind::Logic(logic) if logic.logic_type == LogicType::Or) + { + continue; + } + for &output_id in &node.outputs { + let Some(output) = self.graph.graph.find_node_by_id(output_id) else { + continue; + }; + if matches!(&output.kind, GraphNodeKind::Logic(logic) if logic.logic_type == LogicType::Or) + { + direct_edges.push((node.id, output.id)); + } + } + } let mut next_id = self.graph.graph.next_node_id(); for (from, to) in direct_edges { @@ -499,24 +494,28 @@ impl LogicGraphTransformer { .graph .replace_target_input_node_ids(to, from, vec![second_not]); - self.graph.graph.insert_node(GraphNode { - id: first_not, - kind: GraphNodeKind::Logic(Logic { - logic_type: LogicType::Not, - }), - inputs: vec![from], - outputs: vec![second_not], - tag: "auto-buffer".to_owned(), - }); - self.graph.graph.insert_node(GraphNode { - id: second_not, - kind: GraphNodeKind::Logic(Logic { - logic_type: LogicType::Not, - }), - inputs: vec![first_not], - outputs: vec![to], - tag: "auto-buffer".to_owned(), - }); + self.graph.graph.insert_node_with_id( + first_not, + GraphNode { + kind: GraphNodeKind::Logic(Logic { + logic_type: LogicType::Not, + }), + inputs: vec![from], + outputs: vec![second_not], + tag: "auto-buffer".to_owned(), + }, + ); + self.graph.graph.insert_node_with_id( + second_not, + GraphNode { + kind: GraphNodeKind::Logic(Logic { + logic_type: LogicType::Not, + }), + inputs: vec![first_not], + outputs: vec![to], + tag: "auto-buffer".to_owned(), + }, + ); } self.graph.graph.build_inputs(); @@ -684,19 +683,16 @@ mod tests { fn prepare_place_preserves_sequential_boundaries() -> eyre::Result<()> { let mut graph = Graph::from_nodes(vec![ GraphNode { - id: 0, kind: GraphNodeKind::Input("s".to_owned()), outputs: vec![2], ..Default::default() }, GraphNode { - id: 1, kind: GraphNodeKind::Input("r".to_owned()), outputs: vec![2], ..Default::default() }, GraphNode { - id: 2, kind: GraphNodeKind::Sequential(SequentialPrimitive::new( SequentialType::RsLatch, vec!["s".to_owned(), "r".to_owned()], @@ -707,7 +703,6 @@ mod tests { ..Default::default() }, GraphNode { - id: 3, kind: GraphNodeKind::Logic(Logic { logic_type: LogicType::Not, }), @@ -716,7 +711,6 @@ mod tests { ..Default::default() }, GraphNode { - id: 4, kind: GraphNodeKind::Output("not_q".to_owned()), inputs: vec![3], ..Default::default() diff --git a/src/transform/place_and_route/local_placer/component_tests.rs b/src/transform/place_and_route/local_placer/component_tests.rs index a7ea6d0..10eb318 100644 --- a/src/transform/place_and_route/local_placer/component_tests.rs +++ b/src/transform/place_and_route/local_placer/component_tests.rs @@ -1,7 +1,7 @@ use crate::graph::analysis::equivalent_expression_groups; use crate::graph::graphviz::ToGraphvizGraph; use crate::graph::logic::predefined_logics; -use crate::graph::{GraphNode, GraphNodeKind}; +use crate::graph::{GraphNode, GraphNodeKind, GraphNodeRef}; use crate::nbt::{NBTRoot, ToNBT}; use crate::sequential::{SequentialPrimitive, SequentialType}; use crate::transform::place_and_route::estimate::world_compact_cost; @@ -304,24 +304,25 @@ fn test_generate_component_rs_latch() -> eyre::Result<()> { vec!["s".to_owned(), "r".to_owned()], vec!["q".to_owned()], ); + let node_id = 2; let node = GraphNode { - id: 2, kind: GraphNodeKind::Sequential(sequential.clone()), inputs: vec![0, 1], ..Default::default() }; + let node_ref = GraphNodeRef::new(node_id, &node); let state = [(0, s), (1, r)].into_iter().collect(); let pairs = select_rs_latch_not_pairs( &config, - &node, + node_ref, &sequential, &state, generate_rs_latch_not_pairs(&world), ); let generated = pairs .into_iter() - .flat_map(|placed| route_rs_latch_branches(&config, &node, &sequential, &state, placed)) + .flat_map(|placed| route_rs_latch_branches(&config, node_ref, &sequential, &state, placed)) .collect::>(); assert!(!generated.is_empty()); let valid = generated.into_iter().find_map(|placed| { @@ -372,15 +373,16 @@ fn test_generate_component_d_latch() -> eyre::Result<()> { vec!["d".to_owned(), "en".to_owned()], vec!["q".to_owned()], ); + let node_id = 2; let node = GraphNode { - id: 2, kind: GraphNodeKind::Sequential(sequential.clone()), inputs: vec![0, 1], ..Default::default() }; + let node_ref = GraphNodeRef::new(node_id, &node); let state = [(0, d), (1, en)].into_iter().collect(); - let generated = generate_d_latch_gate_routes(&config, &node, &sequential, &world, &state); + let generated = generate_d_latch_gate_routes(&config, node_ref, &sequential, &world, &state); assert!(!generated.is_empty()); let mut checked = 0usize; let valid = generated.iter().find_map(|(world, state)| { @@ -394,7 +396,7 @@ fn test_generate_component_d_latch() -> eyre::Result<()> { eprintln!("candidate {checked} failed: {error}; q={q:?} nq={nq:?}"); let q_support = q.walk(world[q].direction).unwrap(); let nq_support = nq.walk(world[nq].direction).unwrap(); - let nodes = rs_latch_input_node_ids(node.id); + let nodes = rs_latch_input_node_ids(node_id); let _ = trace_d_latch_behavior( world, d, diff --git a/src/transform/place_and_route/local_placer/mod.rs b/src/transform/place_and_route/local_placer/mod.rs index 2fde7c2..562c7d2 100644 --- a/src/transform/place_and_route/local_placer/mod.rs +++ b/src/transform/place_and_route/local_placer/mod.rs @@ -10,7 +10,7 @@ use super::place_bound::PlaceBound; use super::placed_node::PlacedNode; use super::sampling::SamplingPolicy; use crate::graph::logic::LogicGraph; -use crate::graph::{GraphNode, GraphNodeId, GraphNodeKind}; +use crate::graph::{GraphNodeId, GraphNodeKind, GraphNodeRef}; use crate::logic::LogicType; use crate::output::{OutputEndpoint, PlacedWorld}; use crate::sequential::layout::SequentialMacro; @@ -291,7 +291,7 @@ impl LocalPlacer { fn generate_place_and_route( &self, - node: &GraphNode, + node: GraphNodeRef<'_>, world: World3D, state: &PlacementState, ) -> PlacementGeneration { @@ -482,7 +482,7 @@ impl LocalPlacer { .filter(|node| { self.config.materialize_outputs || !matches!(node.kind, GraphNodeKind::Output(_)) }) - .flat_map(|node| node.inputs.iter().copied()) + .flat_map(|node| node.inputs.clone()) .chain(self.graph.externally_observable_output_source_ids()) .chain( self.config diff --git a/src/transform/place_and_route/local_placer/sequential/d_latch.rs b/src/transform/place_and_route/local_placer/sequential/d_latch.rs index 43e23e6..df8ff30 100644 --- a/src/transform/place_and_route/local_placer/sequential/d_latch.rs +++ b/src/transform/place_and_route/local_placer/sequential/d_latch.rs @@ -4,6 +4,7 @@ use super::prefix::RsLatchPrefixPlan; use super::rs_latch::generate_rs_latch_gate_routes; use super::scenario::{run_sequential_scenario, torch_output_value, SequentialScenarioStep}; use super::*; +use crate::graph::GraphNode; use crate::world::simulator::Simulator; use crate::world::World; @@ -19,7 +20,7 @@ const D_LATCH_SIM_TRACE_LIMIT: usize = 0; pub(in super::super) fn generate_d_latch_gate_routes( config: &LocalPlacerConfig, - node: &GraphNode, + node: GraphNodeRef<'_>, sequential: &SequentialPrimitive, world: &World3D, state: &PlacementState, @@ -37,11 +38,11 @@ pub(in super::super) fn generate_d_latch_gate_routes( sequential.output_ports.clone(), ); let rs_node = GraphNode { - id: node.id, kind: GraphNodeKind::Sequential(rs_sequential.clone()), inputs: prefix_plan.rs_node_inputs(), ..Default::default() }; + let rs_node = GraphNodeRef::new(node.id, &rs_node); let queue = prefix_plan.place(config, world, state); let queue = retain_valid_d_latch_prefix(config, queue, &prefix_plan); @@ -52,7 +53,7 @@ pub(in super::super) fn generate_d_latch_gate_routes( &prefix_plan, data_input, enable_input, - &rs_node, + rs_node, &rs_sequential, queue, ); @@ -65,12 +66,12 @@ pub(in super::super) fn generate_d_latch_gate_routes( fn route_d_latch_core( config: &LocalPlacerConfig, - node: &GraphNode, + node: GraphNodeRef<'_>, outer_state: &PlacementState, prefix_plan: &RsLatchPrefixPlan, data_input: GraphNodeId, enable_input: GraphNodeId, - rs_node: &GraphNode, + rs_node: GraphNodeRef<'_>, rs_sequential: &SequentialPrimitive, queue: PlacerQueue, ) -> PlacerQueue { diff --git a/src/transform/place_and_route/local_placer/sequential/macro_routes.rs b/src/transform/place_and_route/local_placer/sequential/macro_routes.rs index b094ef0..a33e3a3 100644 --- a/src/transform/place_and_route/local_placer/sequential/macro_routes.rs +++ b/src/transform/place_and_route/local_placer/sequential/macro_routes.rs @@ -6,7 +6,7 @@ use super::*; pub(in super::super) fn generate_sequential_macro_routes( config: &LocalPlacerConfig, - node: &GraphNode, + node: GraphNodeRef<'_>, sequential: &SequentialPrimitive, world: &World3D, state: &PlacementState, @@ -73,7 +73,7 @@ pub(in super::super) fn place_sequential_macro( pub(in super::super) fn route_sequential_inputs( config: &LocalPlacerConfig, - node: &GraphNode, + node: GraphNodeRef<'_>, sequential: &SequentialPrimitive, state: &PlacementState, placed: &PlacedSequentialMacro, diff --git a/src/transform/place_and_route/local_placer/sequential/prefix.rs b/src/transform/place_and_route/local_placer/sequential/prefix.rs index 19a3a30..a72124a 100644 --- a/src/transform/place_and_route/local_placer/sequential/prefix.rs +++ b/src/transform/place_and_route/local_placer/sequential/prefix.rs @@ -15,7 +15,7 @@ pub(super) struct RsLatchPrefixPlan { impl RsLatchPrefixPlan { pub(super) fn build( - node: &GraphNode, + node: GraphNodeRef<'_>, sequential: &SequentialPrimitive, state: &PlacementState, ) -> Option { @@ -29,14 +29,14 @@ impl RsLatchPrefixPlan { let mut input_node_ids_by_port = HashMap::new(); let mut input_positions_by_port = HashMap::new(); for prefix_input in graph.nodes.iter().filter_map(|node| match &node.kind { - GraphNodeKind::Input(name) => Some((node.id, name.as_str())), + GraphNodeKind::Input(name) => Some((node.id, name.clone())), _ => None, }) { - let outer_input = outer_input_node_id(node, sequential, prefix_input.1)?; + let outer_input = outer_input_node_id(node, sequential, &prefix_input.1)?; let position = state.node_position(outer_input)?; input_mappings.push((prefix_input.0, outer_input)); - input_node_ids_by_port.insert(prefix_input.1.to_owned(), outer_input); - input_positions_by_port.insert(prefix_input.1.to_owned(), position); + input_node_ids_by_port.insert(prefix_input.1.clone(), outer_input); + input_positions_by_port.insert(prefix_input.1, position); } Some(Self { @@ -115,7 +115,7 @@ impl RsLatchPrefixPlan { } fn outer_input_node_id( - node: &GraphNode, + node: GraphNodeRef<'_>, sequential: &SequentialPrimitive, port: &str, ) -> Option { diff --git a/src/transform/place_and_route/local_placer/sequential/rs_latch.rs b/src/transform/place_and_route/local_placer/sequential/rs_latch.rs index c75db1e..9a70af5 100644 --- a/src/transform/place_and_route/local_placer/sequential/rs_latch.rs +++ b/src/transform/place_and_route/local_placer/sequential/rs_latch.rs @@ -28,7 +28,7 @@ pub(in super::super) struct RsLatchGatePlacement { pub(in super::super) fn generate_rs_latch_gate_routes( config: &LocalPlacerConfig, - node: &GraphNode, + node: GraphNodeRef<'_>, sequential: &SequentialPrimitive, world: &World3D, state: &PlacementState, @@ -72,7 +72,7 @@ pub(in super::super) fn generate_rs_latch_gate_routes( pub(in super::super) fn select_rs_latch_not_pairs( config: &LocalPlacerConfig, - node: &GraphNode, + node: GraphNodeRef<'_>, sequential: &SequentialPrimitive, state: &PlacementState, mut pairs: Vec, @@ -130,7 +130,7 @@ fn lies_between(start: Position, end: Position, position: Position) -> bool { pub(in super::super) fn route_rs_latch_branches( config: &LocalPlacerConfig, - node: &GraphNode, + node: GraphNodeRef<'_>, sequential: &SequentialPrimitive, state: &PlacementState, placed: RsLatchGatePlacement, @@ -259,7 +259,7 @@ fn redstone_route_network( } fn rs_latch_input_source( - node: &GraphNode, + node: GraphNodeRef<'_>, sequential: &SequentialPrimitive, state: &PlacementState, port: &str, diff --git a/src/transform/place_and_route/local_placer/tests.rs b/src/transform/place_and_route/local_placer/tests.rs index 35aa5f6..b6b2ff8 100644 --- a/src/transform/place_and_route/local_placer/tests.rs +++ b/src/transform/place_and_route/local_placer/tests.rs @@ -1,6 +1,6 @@ use super::*; use crate::graph::logic::{predefined_logics, LogicGraph}; -use crate::graph::{Graph, GraphNode, GraphNodeKind}; +use crate::graph::{Graph, GraphNode, GraphNodeKind, GraphNodeRef}; use crate::logic::LogicType; use crate::sequential::layout::SequentialMacro; use crate::sequential::{SequentialPrimitive, SequentialType}; @@ -345,32 +345,26 @@ fn generate_with_outputs_reports_materialized_output_positions() -> eyre::Result fn future_join_cost_weights_pairs_with_remaining_fanout() { let mut graph = Graph::from_nodes(vec![ GraphNode { - id: 0, kind: GraphNodeKind::Input("a".to_owned()), ..Default::default() }, GraphNode { - id: 1, kind: GraphNodeKind::Input("b".to_owned()), ..Default::default() }, GraphNode { - id: 2, kind: GraphNodeKind::Input("c".to_owned()), ..Default::default() }, GraphNode { - id: 3, kind: GraphNodeKind::Input("d".to_owned()), ..Default::default() }, GraphNode { - id: 4, kind: GraphNodeKind::Input("e".to_owned()), ..Default::default() }, GraphNode { - id: 5, kind: GraphNodeKind::Logic(crate::logic::Logic { logic_type: LogicType::Or, }), @@ -378,7 +372,6 @@ fn future_join_cost_weights_pairs_with_remaining_fanout() { ..Default::default() }, GraphNode { - id: 6, kind: GraphNodeKind::Logic(crate::logic::Logic { logic_type: LogicType::Or, }), @@ -386,7 +379,6 @@ fn future_join_cost_weights_pairs_with_remaining_fanout() { ..Default::default() }, GraphNode { - id: 7, kind: GraphNodeKind::Logic(crate::logic::Logic { logic_type: LogicType::Or, }), @@ -791,19 +783,16 @@ fn generate_or_routes_finds_adjacent_torch_route() { fn local_placer_accepts_q_only_sequential_primitives_with_macro_candidates() { let mut graph = Graph::from_nodes(vec![ GraphNode { - id: 0, kind: GraphNodeKind::Input("s".to_owned()), outputs: vec![2], ..Default::default() }, GraphNode { - id: 1, kind: GraphNodeKind::Input("r".to_owned()), outputs: vec![2], ..Default::default() }, GraphNode { - id: 2, kind: GraphNodeKind::Sequential(SequentialPrimitive::new( SequentialType::RsLatch, vec!["s".to_owned(), "r".to_owned()], @@ -814,7 +803,6 @@ fn local_placer_accepts_q_only_sequential_primitives_with_macro_candidates() { ..Default::default() }, GraphNode { - id: 3, kind: GraphNodeKind::Output("q".to_owned()), inputs: vec![2], ..Default::default() @@ -831,7 +819,6 @@ fn local_placer_accepts_q_only_sequential_primitives_with_macro_candidates() { #[test] fn local_placer_rejects_multi_output_sequential_primitives_until_edges_are_port_aware() { let mut graph = Graph::from_nodes(vec![GraphNode { - id: 0, kind: GraphNodeKind::Sequential(SequentialPrimitive::rs_latch()), outputs: vec![1], ..Default::default() @@ -852,7 +839,6 @@ fn local_placer_rejects_multi_output_sequential_primitives_until_edges_are_port_ #[test] fn local_placer_rejects_sequential_primitives_without_macro_candidates() { let mut graph = Graph::from_nodes(vec![GraphNode { - id: 0, kind: GraphNodeKind::Sequential(SequentialPrimitive::new( SequentialType::DFlipFlop, Vec::new(), @@ -876,8 +862,8 @@ fn local_placer_rejects_sequential_primitives_without_macro_candidates() { #[test] fn sequential_macro_generation_registers_output_port_positions() { + let node_id = 7; let node = GraphNode { - id: 7, kind: GraphNodeKind::Sequential(SequentialPrimitive::new( SequentialType::RsLatch, Vec::new(), @@ -888,18 +874,19 @@ fn sequential_macro_generation_registers_output_port_positions() { let GraphNodeKind::Sequential(sequential) = &node.kind else { panic!("expected sequential node"); }; + let node_ref = GraphNodeRef::new(node_id, &node); let generated = generate_sequential_macro_routes( &config(1), - &node, + node_ref, sequential, &World3D::new(DimSize(8, 8, 4)), &PlacementState::default(), ); assert!(!generated.is_empty()); - assert!(generated[0].1.node_position(node.id).is_some()); - assert!(generated[0].1.port_position(node.id, "q").is_some()); + assert!(generated[0].1.node_position(node_id).is_some()); + assert!(generated[0].1.port_position(node_id, "q").is_some()); } #[test] @@ -922,16 +909,16 @@ fn sequential_macro_routes_input_ports_from_existing_sources() { vec!["q".to_owned()], ); let node = GraphNode { - id: 2, kind: GraphNodeKind::Sequential(sequential.clone()), inputs: vec![0, 1], ..Default::default() }; + let node_ref = GraphNodeRef::new(2, &node); let candidate = SequentialMacro::candidates(&sequential).remove(0); let placed = place_sequential_macro(&world, &candidate, Position(2, 1, 0)).unwrap(); let state = [(0, source_s), (1, source_r)].into_iter().collect(); - let routed = route_sequential_inputs(&config(4), &node, &sequential, &state, &placed); + let routed = route_sequential_inputs(&config(4), node_ref, &sequential, &state, &placed); assert!(!routed.is_empty()); } diff --git a/src/transform/world.rs b/src/transform/world.rs index 5a9cc0e..e97f10b 100644 --- a/src/transform/world.rs +++ b/src/transform/world.rs @@ -95,8 +95,8 @@ impl WorldGraphTransformer { } for group_id in group_ids.iter().sorted() { + let node_id = next_id; let node = GraphNode { - id: next_id, kind: GraphNodeKind::Block(Block { kind: BlockKind::Redstone { on_count: 0, @@ -121,7 +121,7 @@ impl WorldGraphTransformer { .find_node_by_id_mut(conn) .unwrap() .outputs - .push(next_id); + .push(node_id); }); group_outputs[group_id].iter().for_each(|&conn| { self.graph @@ -129,11 +129,11 @@ impl WorldGraphTransformer { .find_node_by_id_mut(conn) .unwrap() .inputs - .push(next_id); + .push(node_id); }); - self.graph.routings.insert(node.id); - self.graph.graph.insert_node(node); + self.graph.routings.insert(node_id); + self.graph.graph.insert_node_with_id(node_id, node); next_id += 1; } diff --git a/src/transform/world_to_logic.rs b/src/transform/world_to_logic.rs index 49f3f67..ae67614 100644 --- a/src/transform/world_to_logic.rs +++ b/src/transform/world_to_logic.rs @@ -99,71 +99,83 @@ impl WorldToLogicTransformer { let new_nodes = match node.kind { GraphNodeKind::Sequential(sequential) => { - vec![GraphNode { - id: node.id, - kind: GraphNodeKind::Sequential(sequential), - inputs, - outputs: node.outputs, - tag, - }] + vec![( + id, + GraphNode { + kind: GraphNodeKind::Sequential(sequential), + inputs, + outputs: node.outputs, + tag, + }, + )] } GraphNodeKind::Block(block) => match block.kind { BlockKind::Switch { .. } => { input_count += 1; - vec![GraphNode { - id: node.id, - kind: GraphNodeKind::Input(format!("#{}", input_count)), - inputs, - outputs: node.outputs, - tag, - }] + vec![( + id, + GraphNode { + kind: GraphNodeKind::Input(format!("#{}", input_count)), + inputs, + outputs: node.outputs, + tag, + }, + )] } BlockKind::Redstone { .. } | BlockKind::Repeater { .. } => { - vec![GraphNode { - id: node.id, - kind: GraphNodeKind::Logic(Logic { - logic_type: LogicType::Or, - }), - inputs, - outputs: node.outputs, - tag, - }] - } - BlockKind::Torch { .. } => { - if node.inputs.len() == 1 { - vec![GraphNode { - id: node.id, + vec![( + id, + GraphNode { kind: GraphNodeKind::Logic(Logic { - logic_type: LogicType::Not, + logic_type: LogicType::Or, }), inputs, outputs: node.outputs, tag, - }] + }, + )] + } + BlockKind::Torch { .. } => { + if node.inputs.len() == 1 { + vec![( + id, + GraphNode { + kind: GraphNodeKind::Logic(Logic { + logic_type: LogicType::Not, + }), + inputs, + outputs: node.outputs, + tag, + }, + )] } else { let not_node_id = self.graph.graph.allocate_node_id(); - let or_node = GraphNode { - id: node.id, - kind: GraphNodeKind::Logic(Logic { - logic_type: LogicType::Or, - }), - inputs, - outputs: vec![not_node_id], - tag: tag.clone(), - }; - - let not_node = GraphNode { - id: not_node_id, - kind: GraphNodeKind::Logic(Logic { - logic_type: LogicType::Not, - }), - inputs: vec![or_node.id], - outputs: node.outputs.clone(), - tag, - }; - - new_in_id.insert(or_node.id, not_node_id); + let or_node = ( + id, + GraphNode { + kind: GraphNodeKind::Logic(Logic { + logic_type: LogicType::Or, + }), + inputs, + outputs: vec![not_node_id], + tag: tag.clone(), + }, + ); + + let not_node = ( + not_node_id, + GraphNode { + kind: GraphNodeKind::Logic(Logic { + logic_type: LogicType::Not, + }), + inputs: vec![id], + outputs: node.outputs.clone(), + tag, + }, + ); + + new_in_id.insert(id, not_node_id); vec![or_node, not_node] } @@ -176,7 +188,7 @@ impl WorldToLogicTransformer { nodes.extend(new_nodes); } - let mut graph = Graph::from_nodes(nodes); + let mut graph = Graph::from_nodes_with_ids(nodes); graph.build_outputs(); graph.build_producers(); @@ -362,25 +374,31 @@ mod tests { kind: BlockKind::Torch { is_on: true }, direction: Direction::Bottom, }; - let mut graph = crate::graph::Graph::from_nodes(vec![ - crate::graph::GraphNode { - id: 10, - kind: crate::graph::GraphNodeKind::Block(switch), - outputs: vec![30], - ..Default::default() - }, - crate::graph::GraphNode { - id: 20, - kind: crate::graph::GraphNodeKind::Block(switch), - outputs: vec![30], - ..Default::default() - }, - crate::graph::GraphNode { - id: 30, - kind: crate::graph::GraphNodeKind::Block(torch), - inputs: vec![10, 20], - ..Default::default() - }, + let mut graph = crate::graph::Graph::from_nodes_with_ids(vec![ + ( + 10, + crate::graph::GraphNode { + kind: crate::graph::GraphNodeKind::Block(switch), + outputs: vec![30], + ..Default::default() + }, + ), + ( + 20, + crate::graph::GraphNode { + kind: crate::graph::GraphNodeKind::Block(switch), + outputs: vec![30], + ..Default::default() + }, + ), + ( + 30, + crate::graph::GraphNode { + kind: crate::graph::GraphNodeKind::Block(torch), + inputs: vec![10, 20], + ..Default::default() + }, + ), ]); graph.build_inputs(); graph.build_outputs(); From 36d37b3e76f895774bfaccfa15ee51793e2d434b Mon Sep 17 00:00:00 2001 From: rollrat Date: Sun, 31 May 2026 14:28:45 +0900 Subject: [PATCH 08/11] Restore graph comments encoding Fix comments that were mojibaked during the GraphNode id refactor. This keeps the code behavior unchanged and restores the original Korean explanatory comments around graph merge and rebuild helpers. --- src/graph/mod.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/graph/mod.rs b/src/graph/mod.rs index 5a6340f..04fdc20 100644 --- a/src/graph/mod.rs +++ b/src/graph/mod.rs @@ -496,7 +496,7 @@ impl Graph { self.consumers.extend(other.consumers); } - // src?€ other??input??媛숈? 寃??쇰━ ?곌껐?섏뿬 merge?? + // src와 other의 input이 같은 것 끼리 연결하여 merge함 pub fn merge_by_input(&mut self, mut other: Self) { if let Some(id) = self.max_node_id() { other.rebuild_node_id_base(id + 1); @@ -543,7 +543,7 @@ impl Graph { self.build_consumers(); } - // src??output??target??input怨??곌껐?섏뿬 merge?? + // src의 output을 target의 input과 연결하여 merge함 pub fn merge_by_outin(&mut self, mut target: Self, out_in: Vec<(&str, &str)>) { if let Some(id) = self.max_node_id() { target.rebuild_node_id_base(id + 1); @@ -598,7 +598,7 @@ impl Graph { self.build_consumers(); } - // self??input, output?ㅼ쓣 target??input怨??곌껐?? + // self의 input, output들을 target의 input과 연결함 pub fn merge(&mut self, mut target: Self) { if let Some(id) = self.max_node_id() { target.rebuild_node_id_base(id + 1); @@ -844,7 +844,7 @@ impl Graph { replacement_id } - // ?몃뱶 ??젣?섍퀬 ??젣???몃뱶??Input Output?쇰━ ?곌껐?? + // 노드 삭제하고 삭제한 노드의 Input Output끼리 연결함 pub fn remove_and_reconnect_by_node_id_lazy(&mut self, node_id: GraphNodeId) { let Some(index) = self.nodes.iter().position(|node| node.id == node_id) else { return; @@ -922,7 +922,7 @@ impl Graph { node.outputs.extend(to); } - // outputs??諛섎뱶??determine ?섏뼱?쇳븿 + // outputs이 반드시 determine 되어야함 pub fn build_inputs(&mut self) { let mut input_map: HashMap> = HashMap::new(); @@ -943,7 +943,7 @@ impl Graph { } } - // inputs??諛섎뱶??determine ?섏뼱?쇳븿 + // inputs이 반드시 determine 되어야함 pub fn build_outputs(&mut self) { let mut output_map: HashMap> = HashMap::new(); From bbbef254aa1121ab6e71cbedd005756cc7ed6c09 Mon Sep 17 00:00:00 2001 From: rollrat Date: Sun, 31 May 2026 15:25:51 +0900 Subject: [PATCH 09/11] Close graph id allocation APIs Keep GraphNodeId production behind Graph::add_node and keyed graph construction. Remove direct keyed insertion and local synthetic id counters from graph builders, transforms, and clustered graph construction so transforms no longer mint graph ids outside Graph. --- src/graph/logic.rs | 36 +++------- src/graph/mod.rs | 73 +++++++------------ src/sequential/core.rs | 19 ++--- src/transform/logic.rs | 84 +++++++++------------- src/transform/world.rs | 6 +- src/transform/world_to_logic.rs | 122 +++++++++++++------------------- 6 files changed, 130 insertions(+), 210 deletions(-) diff --git a/src/graph/logic.rs b/src/graph/logic.rs index 753ba0f..ff4c5c3 100644 --- a/src/graph/logic.rs +++ b/src/graph/logic.rs @@ -98,21 +98,16 @@ impl LogicGraph { where I: IntoIterator, { - let mut next_id = self.graph.next_node_id(); for (name, source_id) in outputs { if self.find_node_by_id(source_id).is_none() { eyre::bail!("cannot attach output {name}: missing source node {source_id}"); } - self.graph.insert_node_with_id( - next_id, - GraphNode { - kind: GraphNodeKind::Output(name), - inputs: vec![source_id], - ..Default::default() - }, - ); - next_id += 1; + self.graph.add_node(GraphNode { + kind: GraphNodeKind::Output(name), + inputs: vec![source_id], + ..Default::default() + }); } self.graph.build_outputs(); @@ -312,9 +307,8 @@ fn permuted_output_table_set( #[derive(Default)] pub struct LogicGraphBuilder { stmt: String, - node_id: usize, ptr: usize, - nodes: Vec<(GraphNodeId, GraphNode)>, + graph: Graph, inputs: HashMap, } @@ -349,16 +343,9 @@ impl LogicGraphBuilder { pub fn build(mut self, output_name: String) -> eyre::Result { self.do_parse(output_name); - let mut graph = Graph::from_nodes_with_ids(self.nodes.clone()); - graph.build_outputs(); - - Ok(LogicGraph { graph }) - } + self.graph.build_outputs(); - fn next_id(&mut self) -> usize { - let id = self.node_id; - self.node_id += 1; - id + Ok(LogicGraph { graph: self.graph }) } fn next_ptr(&mut self) -> usize { @@ -368,14 +355,11 @@ impl LogicGraphBuilder { } fn new_node(&mut self, kind: GraphNodeKind, inputs: Vec) -> GraphNodeId { - let id = self.next_id(); - let node = GraphNode { + self.graph.add_node(GraphNode { kind, inputs, ..Default::default() - }; - self.nodes.push((id, node)); - id + }) } fn new_input_node(&mut self, name: String) -> GraphNodeId { diff --git a/src/graph/mod.rs b/src/graph/mod.rs index 04fdc20..e8b73b6 100644 --- a/src/graph/mod.rs +++ b/src/graph/mod.rs @@ -277,7 +277,7 @@ impl GraphNodes { .for_each(|entry| self.insert_with_id(entry.id, entry.node)); } - pub fn insert_with_id(&mut self, id: GraphNodeId, node: GraphNode) { + fn insert_with_id(&mut self, id: GraphNodeId, node: GraphNode) { self.nodes.insert(id, node); } } @@ -724,12 +724,12 @@ impl Graph { .map(|node| node.id) } - pub fn next_node_id(&self) -> GraphNodeId { + fn next_node_id(&self) -> GraphNodeId { self.next_node_id .max(self.max_node_id().map_or(0, |id| id + 1)) } - pub fn allocate_node_id(&mut self) -> GraphNodeId { + fn allocate_node_id(&mut self) -> GraphNodeId { let id = self.next_node_id(); self.next_node_id = id + 1; id @@ -737,13 +737,8 @@ impl Graph { pub fn add_node(&mut self, node: GraphNode) -> GraphNodeId { let id = self.allocate_node_id(); - self.insert_node_with_id(id, node); - id - } - - pub(crate) fn insert_node_with_id(&mut self, id: GraphNodeId, node: GraphNode) { - self.next_node_id = self.next_node_id.max(id + 1); self.nodes.insert_with_id(id, node); + id } pub fn find_node_by_input_name(&mut self, input_name: &str) -> Option> { @@ -805,7 +800,7 @@ impl Graph { outputs: Vec, tag: String, ) -> GraphNodeId { - let replacement_id = self.next_node_id(); + let replacement_id = self.allocate_node_id(); for input in &inputs { if let Some(mut node) = self.find_node_by_id_mut(*input) { @@ -827,7 +822,7 @@ impl Graph { self.remove_by_node_id_lazy(*node_id); } - self.insert_node_with_id( + self.nodes.insert_with_id( replacement_id, GraphNode { kind, @@ -1259,7 +1254,10 @@ impl<'a> From>> for Graph { } } -pub fn subgraphs_to_clustered_graph(graph: &Graph, subgraphs: &[SubGraph]) -> ClusteredGraph { +pub fn subgraphs_to_clustered_graph( + source_graph: &Graph, + subgraphs: &[SubGraph], +) -> ClusteredGraph { // GraphNodeId, Cluster Index let cluster_index: HashMap = subgraphs .iter() @@ -1267,7 +1265,7 @@ pub fn subgraphs_to_clustered_graph(graph: &Graph, subgraphs: &[SubGraph]) -> Cl .flat_map(|(index, sg)| sg.nodes.iter().map(|&id| (id, index)).collect_vec()) .collect(); - let mut nodes = subgraphs + let nodes = subgraphs .iter() .enumerate() .map(|(index, sg)| { @@ -1284,6 +1282,7 @@ pub fn subgraphs_to_clustered_graph(graph: &Graph, subgraphs: &[SubGraph]) -> Cl ) }) .collect_vec(); + let mut clustered_graph = Graph::from_nodes_with_ids(nodes); let weighted_node = subgraphs .iter() @@ -1292,7 +1291,7 @@ pub fn subgraphs_to_clustered_graph(graph: &Graph, subgraphs: &[SubGraph]) -> Cl let consumers = sg .nodes .iter() - .flat_map(|node| graph.consumers[&node.id()].clone()) + .flat_map(|node| source_graph.consumers[&node.id()].clone()) .collect_vec(); // cluster_index, weight @@ -1323,35 +1322,34 @@ pub fn subgraphs_to_clustered_graph(graph: &Graph, subgraphs: &[SubGraph]) -> Cl }) .collect_vec(); - let weighted_base_id = nodes.len(); - let mut weighted_nodes = Vec::new(); - let mut weighted_index = 0; let mut weighted_outputs = Vec::new(); for nodes in weighted_node { let mut outputs = Vec::new(); for node in nodes { - let id = weighted_base_id + weighted_index + 1; + let id = clustered_graph.add_node(node); outputs.push(id); - weighted_nodes.push((id, node)); - weighted_index += 1; } weighted_outputs.push(outputs); } // Set output weighted node - for (index, (_, clustered_node)) in nodes.iter_mut().enumerate() { + for (index, mut clustered_node) in clustered_graph + .nodes + .iter_mut() + .take(weighted_outputs.len()) + .enumerate() + { clustered_node.outputs = weighted_outputs[index].clone(); } - nodes.extend(weighted_nodes); - let mut graph = Graph::from_nodes_with_ids(nodes); - - graph.build_inputs(); - graph.build_producers(); - graph.build_consumers(); - graph.verify().unwrap(); + clustered_graph.build_inputs(); + clustered_graph.build_producers(); + clustered_graph.build_consumers(); + clustered_graph.verify().unwrap(); - ClusteredGraph { graph } + ClusteredGraph { + graph: clustered_graph, + } } #[cfg(test)] @@ -1529,23 +1527,6 @@ mod tests { assert_eq!(petgraph.node_count(), 3); assert_eq!(petgraph.edge_count(), 2); - graph.insert_node_with_id( - 25, - GraphNode { - tag: "replacement".to_owned(), - ..Default::default() - }, - ); - assert_eq!( - graph.nodes.iter().map(|node| node.id).collect_vec(), - vec![10, 20, 30, 25] - ); - assert_eq!(graph.find_node_by_id(25).unwrap().tag, "replacement"); - assert_eq!( - graph.find_and_remove_node_by_id(25).unwrap().tag, - "replacement" - ); - graph.remove_by_node_id_lazy(30); let id = graph.add_node(GraphNode::default()); diff --git a/src/sequential/core.rs b/src/sequential/core.rs index cd3ad16..ba497ae 100644 --- a/src/sequential/core.rs +++ b/src/sequential/core.rs @@ -101,7 +101,7 @@ pub fn rs_latch_prefix_graph(graph: &Graph, core: &RsLatchCore) -> Option Option>(); - let mut next_node_id = graph.next_node_id(); + let mut graph = Graph::from_nodes_with_ids(nodes); for (output_name, source_node_id) in output_roots { - nodes.push(( - next_node_id, - GraphNode { - kind: GraphNodeKind::Output(output_name.to_owned()), - inputs: vec![source_node_id], - ..Default::default() - }, - )); - next_node_id += 1; + graph.add_node(GraphNode { + kind: GraphNodeKind::Output(output_name.to_owned()), + inputs: vec![source_node_id], + ..Default::default() + }); } - let mut graph = Graph::from_nodes_with_ids(nodes); graph.build_outputs(); graph.build_producers(); graph.build_consumers(); diff --git a/src/transform/logic.rs b/src/transform/logic.rs index be63a66..f003b76 100644 --- a/src/transform/logic.rs +++ b/src/transform/logic.rs @@ -308,17 +308,11 @@ impl LogicGraphTransformer { )? .simplify(); - let mut nodes = Vec::new(); - fn make_rc_form( - nodes: &mut Vec<(GraphNodeId, GraphNode)>, - id: &mut usize, + graph: &mut Graph, lookup: &Vec<(GraphNodeId, GraphNode)>, node: &quine_mc_cluskey::Bool, ) -> GraphNodeId { - let tid = *id; - *id += 1; - let node = match node { quine_mc_cluskey::Bool::Term(v) => GraphNode { kind: GraphNodeKind::Input(lookup[*v as usize].1.kind.as_input().to_owned()), @@ -330,7 +324,7 @@ impl LogicGraphTransformer { }), inputs: op .iter() - .map(|v| make_rc_form(nodes, id, lookup, v)) + .map(|v| make_rc_form(graph, lookup, v)) .collect_vec(), ..Default::default() }, @@ -340,7 +334,7 @@ impl LogicGraphTransformer { }), inputs: op .iter() - .map(|v| make_rc_form(nodes, id, lookup, v)) + .map(|v| make_rc_form(graph, lookup, v)) .collect_vec(), ..Default::default() }, @@ -348,29 +342,26 @@ impl LogicGraphTransformer { kind: GraphNodeKind::Logic(Logic { logic_type: LogicType::Not, }), - inputs: vec![make_rc_form(nodes, id, lookup, v)], + inputs: vec![make_rc_form(graph, lookup, v)], ..Default::default() }, _ => unreachable!(), }; if let GraphNodeKind::Input(name) = &node.kind { - if let Some((node_id, _)) = nodes.iter().find( - |(_, node)| matches!(&node.kind, GraphNodeKind::Input(other) if name == other), + if let Some(existing) = graph.nodes.iter().find( + |existing| matches!(&existing.kind, GraphNodeKind::Input(other) if name == other), ) { - *id -= 1; - return *node_id; + return existing.id; } } - nodes.push((tid, node)); - tid + graph.add_node(node) } - let mut id = 0; - let node = make_rc_form(&mut nodes, &mut id, &input_nodes, &results[0]); - let output_id = nodes.len(); - let output_node = GraphNode { + let mut graph = Graph::default(); + let node = make_rc_form(&mut graph, &input_nodes, &results[0]); + graph.add_node(GraphNode { kind: GraphNodeKind::Output( self.graph .graph @@ -382,9 +373,8 @@ impl LogicGraphTransformer { ), inputs: vec![node], ..Default::default() - }; - nodes.push((output_id, output_node)); - self.graph.graph = Graph::from_nodes_with_ids(nodes); + }); + self.graph.graph = graph; self.graph.graph.build_outputs(); self.graph.graph.build_producers(); self.graph.graph.build_consumers(); @@ -481,11 +471,28 @@ impl LogicGraphTransformer { } } - let mut next_id = self.graph.graph.next_node_id(); for (from, to) in direct_edges { - let first_not = next_id; - let second_not = next_id + 1; - next_id += 2; + let first_not = self.graph.graph.add_node(GraphNode { + kind: GraphNodeKind::Logic(Logic { + logic_type: LogicType::Not, + }), + inputs: vec![from], + tag: "auto-buffer".to_owned(), + ..Default::default() + }); + let second_not = self.graph.graph.add_node(GraphNode { + kind: GraphNodeKind::Logic(Logic { + logic_type: LogicType::Not, + }), + inputs: vec![first_not], + outputs: vec![to], + tag: "auto-buffer".to_owned(), + }); + self.graph + .graph + .find_node_by_id_mut(first_not) + .unwrap() + .outputs = vec![second_not]; self.graph .graph @@ -493,29 +500,6 @@ impl LogicGraphTransformer { self.graph .graph .replace_target_input_node_ids(to, from, vec![second_not]); - - self.graph.graph.insert_node_with_id( - first_not, - GraphNode { - kind: GraphNodeKind::Logic(Logic { - logic_type: LogicType::Not, - }), - inputs: vec![from], - outputs: vec![second_not], - tag: "auto-buffer".to_owned(), - }, - ); - self.graph.graph.insert_node_with_id( - second_not, - GraphNode { - kind: GraphNodeKind::Logic(Logic { - logic_type: LogicType::Not, - }), - inputs: vec![first_not], - outputs: vec![to], - tag: "auto-buffer".to_owned(), - }, - ); } self.graph.graph.build_inputs(); diff --git a/src/transform/world.rs b/src/transform/world.rs index e97f10b..276c052 100644 --- a/src/transform/world.rs +++ b/src/transform/world.rs @@ -83,8 +83,6 @@ impl WorldGraphTransformer { ); } - // make clustered node - let mut next_id = self.graph.graph.next_node_id(); for members in group_members.values_mut() { members.sort_unstable(); } @@ -95,7 +93,6 @@ impl WorldGraphTransformer { } for group_id in group_ids.iter().sorted() { - let node_id = next_id; let node = GraphNode { kind: GraphNodeKind::Block(Block { kind: BlockKind::Redstone { @@ -114,6 +111,7 @@ impl WorldGraphTransformer { ), ..Default::default() }; + let node_id = self.graph.graph.add_node(node); group_inputs[group_id].iter().for_each(|&conn| { self.graph @@ -133,8 +131,6 @@ impl WorldGraphTransformer { }); self.graph.routings.insert(node_id); - self.graph.graph.insert_node_with_id(node_id, node); - next_id += 1; } self.graph.graph.build_inputs(); diff --git a/src/transform/world_to_logic.rs b/src/transform/world_to_logic.rs index ae67614..eabdcf8 100644 --- a/src/transform/world_to_logic.rs +++ b/src/transform/world_to_logic.rs @@ -82,12 +82,19 @@ impl WorldToLogicTransformer { fn transform_inner(&mut self) -> eyre::Result { let mut new_in_id = HashMap::new(); - let mut nodes = Vec::new(); + let mut graph = Graph::from_nodes_with_ids( + self.graph + .graph + .nodes + .iter() + .map(|node| (node.id, GraphNode::default())) + .collect(), + ); let mut input_count = 0; for id in self.graph.graph.topological_order() { - let node = self.graph.graph.find_and_remove_node_by_id(id).unwrap(); + let node = self.graph.graph.find_node_by_id(id).unwrap().clone_node(); let inputs = node .inputs @@ -97,87 +104,62 @@ impl WorldToLogicTransformer { .collect(); let tag = transformed_tag(id, &node.tag); - let new_nodes = match node.kind { - GraphNodeKind::Sequential(sequential) => { - vec![( - id, - GraphNode { - kind: GraphNodeKind::Sequential(sequential), - inputs, - outputs: node.outputs, - tag, - }, - )] - } + let new_node = match node.kind { + GraphNodeKind::Sequential(sequential) => GraphNode { + kind: GraphNodeKind::Sequential(sequential), + inputs, + outputs: node.outputs, + tag, + }, GraphNodeKind::Block(block) => match block.kind { BlockKind::Switch { .. } => { input_count += 1; - vec![( - id, - GraphNode { - kind: GraphNodeKind::Input(format!("#{}", input_count)), - inputs, - outputs: node.outputs, - tag, - }, - )] + GraphNode { + kind: GraphNodeKind::Input(format!("#{}", input_count)), + inputs, + outputs: node.outputs, + tag, + } } - BlockKind::Redstone { .. } | BlockKind::Repeater { .. } => { - vec![( - id, + BlockKind::Redstone { .. } | BlockKind::Repeater { .. } => GraphNode { + kind: GraphNodeKind::Logic(Logic { + logic_type: LogicType::Or, + }), + inputs, + outputs: node.outputs, + tag, + }, + BlockKind::Torch { .. } => { + if node.inputs.len() == 1 { GraphNode { kind: GraphNodeKind::Logic(Logic { - logic_type: LogicType::Or, + logic_type: LogicType::Not, }), inputs, outputs: node.outputs, tag, - }, - )] - } - BlockKind::Torch { .. } => { - if node.inputs.len() == 1 { - vec![( - id, - GraphNode { - kind: GraphNodeKind::Logic(Logic { - logic_type: LogicType::Not, - }), - inputs, - outputs: node.outputs, - tag, - }, - )] + } } else { - let not_node_id = self.graph.graph.allocate_node_id(); - let or_node = ( - id, - GraphNode { - kind: GraphNodeKind::Logic(Logic { - logic_type: LogicType::Or, - }), - inputs, - outputs: vec![not_node_id], - tag: tag.clone(), - }, - ); - - let not_node = ( - not_node_id, - GraphNode { - kind: GraphNodeKind::Logic(Logic { - logic_type: LogicType::Not, - }), - inputs: vec![id], - outputs: node.outputs.clone(), - tag, - }, - ); + let not_node_id = graph.add_node(GraphNode { + kind: GraphNodeKind::Logic(Logic { + logic_type: LogicType::Not, + }), + inputs: vec![id], + outputs: node.outputs.clone(), + tag: tag.clone(), + }); new_in_id.insert(id, not_node_id); - vec![or_node, not_node] + GraphNode { + kind: GraphNodeKind::Logic(Logic { + logic_type: LogicType::Or, + }), + inputs, + outputs: vec![not_node_id], + tag: tag.clone(), + } } } _ => todo!(), @@ -185,11 +167,9 @@ impl WorldToLogicTransformer { _ => todo!(), }; - nodes.extend(new_nodes); + *graph.find_node_by_id_mut(id).unwrap() = new_node; } - let mut graph = Graph::from_nodes_with_ids(nodes); - graph.build_outputs(); graph.build_producers(); graph.build_consumers(); From b5884fe8fa866983d35a2c772a5287b69f7179c2 Mon Sep 17 00:00:00 2001 From: rollrat Date: Sun, 31 May 2026 16:08:12 +0900 Subject: [PATCH 10/11] Remove graph node id rebuild Move graph merging to allocator-backed imports so imported nodes receive fresh ids and internal edges are remapped through explicit old-to-new mappings. This keeps GraphNodeId production inside GraphNodes and removes rebuild-style id shifting from graph merges and logic binop replacement. --- src/graph/mod.rs | 331 +++++++++++++++++++++++++++-------------- src/transform/logic.rs | 29 ++-- 2 files changed, 233 insertions(+), 127 deletions(-) diff --git a/src/graph/mod.rs b/src/graph/mod.rs index e8b73b6..da80108 100644 --- a/src/graph/mod.rs +++ b/src/graph/mod.rs @@ -216,6 +216,7 @@ impl std::ops::DerefMut for GraphNodeEntry { #[derive(Default, Debug, Clone)] pub struct GraphNodes { nodes: IndexMap, + next_node_id: GraphNodeId, } impl GraphNodes { @@ -265,28 +266,35 @@ impl GraphNodes { .expect("node index must exist") } - pub fn shift_id(&mut self, from: GraphNodeId, to: GraphNodeId) { - if let Some(node) = self.nodes.shift_remove(&from) { - self.nodes.insert(to, node); - } + fn insert_with_id(&mut self, id: GraphNodeId, node: GraphNode) { + self.next_node_id = self.next_node_id.max(id + 1); + self.nodes.insert(id, node); } - pub fn extend(&mut self, nodes: impl IntoIterator) { - nodes - .into_iter() - .for_each(|entry| self.insert_with_id(entry.id, entry.node)); + fn next_node_id(&self) -> GraphNodeId { + self.next_node_id } - fn insert_with_id(&mut self, id: GraphNodeId, node: GraphNode) { - self.nodes.insert(id, node); + fn allocate_node_id(&mut self) -> GraphNodeId { + let id = self.next_node_id(); + self.next_node_id = id + 1; + id + } + + fn add(&mut self, node: GraphNode) -> GraphNodeId { + let id = self.allocate_node_id(); + self.insert_with_id(id, node); + id } } impl From> for GraphNodes { fn from(mut nodes: Vec<(GraphNodeId, GraphNode)>) -> Self { + let next_node_id = nodes.iter().map(|(id, _)| *id).max().map_or(0, |id| id + 1); nodes.sort_by_key(|(id, _)| *id); Self { nodes: nodes.into_iter().collect(), + next_node_id, } } } @@ -325,17 +333,14 @@ impl<'a> IntoIterator for &'a mut GraphNodes { #[derive(Default, Debug, Clone)] pub struct Graph { pub nodes: GraphNodes, - pub(crate) next_node_id: GraphNodeId, pub producers: HashMap>, pub consumers: HashMap>, } impl Graph { pub fn from_nodes_with_ids(nodes: Vec<(GraphNodeId, GraphNode)>) -> Self { - let next_node_id = nodes.iter().map(|(id, _)| *id).max().map_or(0, |id| id + 1); Self { nodes: nodes.into(), - next_node_id, ..Default::default() } } @@ -486,22 +491,14 @@ impl Graph { self.nodes.iter().map(|node| node.id).collect() } - pub fn concat(&mut self, mut other: Self) { - if let Some(id) = self.max_node_id() { - other.rebuild_node_id_base(id + 1); - } - - self.nodes.extend(other.nodes); - self.producers.extend(other.producers); - self.consumers.extend(other.consumers); + pub fn concat(&mut self, other: Self) { + self.append_node_entries_with_mapping(other.nodes, &HashMap::new()); + self.build_producers(); + self.build_consumers(); } // src와 other의 input이 같은 것 끼리 연결하여 merge함 - pub fn merge_by_input(&mut self, mut other: Self) { - if let Some(id) = self.max_node_id() { - other.rebuild_node_id_base(id + 1); - } - + pub fn merge_by_input(&mut self, other: Self) { let src_inputs: HashMap = self .nodes .iter() @@ -511,44 +508,46 @@ impl Graph { }) .collect(); - let replace_targets: Vec<(GraphNodeId, GraphNodeId)> = other + let shared_inputs: Vec<(GraphNodeId, GraphNodeId, Vec)> = other .nodes .iter() .filter_map(|node| match &node.kind { - GraphNodeKind::Input(name) if src_inputs.contains_key(name) => { - Some((node.id, *src_inputs.get(name).unwrap())) - } + GraphNodeKind::Input(name) if src_inputs.contains_key(name) => Some(( + node.id, + *src_inputs.get(name).unwrap(), + node.outputs.clone(), + )), _ => None, }) .collect(); + let external_ids = shared_inputs + .iter() + .map(|(from, to, _)| (*from, *to)) + .collect::>(); - for (from, to) in replace_targets { - let outputs = other.find_node_by_id(from).unwrap().outputs.clone(); + let imported_ids = self.append_node_entries_with_mapping( + other + .nodes + .into_iter() + .filter(|node| !external_ids.contains_key(&node.id)), + &external_ids, + ); + for (_, to, outputs) in shared_inputs { + let outputs = Self::translate_imported_node_ids(&outputs, &imported_ids, &external_ids); self.find_node_by_id_mut(to) .unwrap() .outputs .extend(outputs); - other.replace_node_id_lazy(from, to); } - other.build_producers(); - other.build_consumers(); - - self.nodes - .extend(other.nodes.into_iter().filter(|node| - !matches!(&node.kind, GraphNodeKind::Input(name) if src_inputs.contains_key(name)))); self.build_inputs(); self.build_producers(); self.build_consumers(); } // src의 output을 target의 input과 연결하여 merge함 - pub fn merge_by_outin(&mut self, mut target: Self, out_in: Vec<(&str, &str)>) { - if let Some(id) = self.max_node_id() { - target.rebuild_node_id_base(id + 1); - } - + pub fn merge_by_outin(&mut self, target: Self, out_in: Vec<(&str, &str)>) { let outputs: HashSet<_> = out_in.iter().map(|(output, _)| *output).collect(); let inputs: HashSet<_> = out_in.iter().map(|(_, input)| *input).collect(); @@ -563,47 +562,50 @@ impl Graph { }) .collect(); - let target_inputs: HashMap> = target + let target_inputs: HashMap)> = target .nodes .iter() .filter_map(|node| match &node.kind { GraphNodeKind::Input(name) if inputs.contains(name.as_str()) => { - Some((name.to_owned(), node.outputs.clone())) + Some((name.to_owned(), (node.id, node.outputs.clone()))) } _ => None, }) .collect(); + let external_ids = out_in + .iter() + .map(|(output, input)| (target_inputs[*input].0, src_outputs[*output])) + .collect::>(); + + let imported_ids = self.append_node_entries_with_mapping( + target + .nodes + .into_iter() + .filter(|node| !external_ids.contains_key(&node.id)), + &external_ids, + ); + + self.nodes.retain(|node| + !matches!(&node.kind, GraphNodeKind::Output(name) if src_outputs.contains_key(name.as_str()))); for (output, input) in out_in { let out_node = src_outputs[output]; - let in_node = target_inputs[input].clone(); - - self.find_node_by_id_mut(out_node).unwrap().outputs = in_node.clone(); + let in_node = Self::translate_imported_node_ids( + &target_inputs[input].1, + &imported_ids, + &external_ids, + ); - for input in in_node { - target.find_node_by_id_mut(input).unwrap().inputs = vec![out_node]; - } + self.find_node_by_id_mut(out_node).unwrap().outputs = in_node; } - target.build_producers(); - target.build_consumers(); - - self.nodes.retain(|node| - !matches!(&node.kind, GraphNodeKind::Output(name) if src_outputs.contains_key(name.as_str()))); - self.nodes - .extend(target.nodes.into_iter().filter(|node| - !matches!(&node.kind, GraphNodeKind::Input(name) if target_inputs.contains_key(name.as_str())))); self.build_inputs(); self.build_producers(); self.build_consumers(); } // self의 input, output들을 target의 input과 연결함 - pub fn merge(&mut self, mut target: Self) { - if let Some(id) = self.max_node_id() { - target.rebuild_node_id_base(id + 1); - } - + pub fn merge(&mut self, target: Self) { let target_inputs: HashSet<_> = target .nodes .iter() @@ -645,19 +647,40 @@ impl Graph { }) .collect(); - let target_inputs: HashMap> = target + let target_inputs: HashMap)> = target .nodes .iter() .filter_map(|node| match &node.kind { GraphNodeKind::Input(name) if targets.contains(name.as_str()) => { - Some((name.to_owned(), node.outputs.clone())) + Some((name.to_owned(), (node.id, node.outputs.clone()))) } _ => None, }) .collect(); + let mut external_ids = HashMap::new(); for name in &targets { - let tar_in = target_inputs[name].clone(); + if let Some(src_in) = src_inputs.get(name) { + external_ids.insert(target_inputs[name].0, *src_in); + } else if let Some(src_out) = src_outputs.get(name) { + external_ids.insert(target_inputs[name].0, *src_out); + } + } + + let imported_ids = self.append_node_entries_with_mapping( + target + .nodes + .into_iter() + .filter(|node| !external_ids.contains_key(&node.id)), + &external_ids, + ); + + for name in &targets { + let tar_in = Self::translate_imported_node_ids( + &target_inputs[name].1, + &imported_ids, + &external_ids, + ); if let Some(src_in) = src_inputs.get(name) { self.find_node_by_id_mut(*src_in) @@ -668,20 +691,10 @@ impl Graph { self.find_node_by_id_mut(*src_out) .unwrap() .outputs - .extend(tar_in.clone()); - - for input in tar_in { - target.find_node_by_id_mut(input).unwrap().inputs = vec![*src_out]; - } + .extend(tar_in); } } - target.build_producers(); - target.build_consumers(); - - self.nodes - .extend(target.nodes.into_iter().filter(|node| - !matches!(&node.kind, GraphNodeKind::Input(name) if target_inputs.contains_key(name.as_str())))); self.build_inputs(); self.build_producers(); self.build_consumers(); @@ -717,28 +730,60 @@ impl Graph { petgraph::algo::is_cyclic_directed(&self.to_petgraph_only_edges()) } - pub fn max_node_id(&self) -> Option { - self.nodes - .iter() - .max_by_key(|node| node.id) - .map(|node| node.id) + pub fn add_node(&mut self, node: GraphNode) -> GraphNodeId { + self.nodes.add(node) } - fn next_node_id(&self) -> GraphNodeId { - self.next_node_id - .max(self.max_node_id().map_or(0, |id| id + 1)) + pub(crate) fn translate_imported_node_id( + node_id: GraphNodeId, + imported_ids: &HashMap, + external_ids: &HashMap, + ) -> GraphNodeId { + external_ids + .get(&node_id) + .or_else(|| imported_ids.get(&node_id)) + .copied() + .unwrap_or(node_id) } - fn allocate_node_id(&mut self) -> GraphNodeId { - let id = self.next_node_id(); - self.next_node_id = id + 1; - id + pub(crate) fn translate_imported_node_ids( + node_ids: &[GraphNodeId], + imported_ids: &HashMap, + external_ids: &HashMap, + ) -> Vec { + node_ids + .iter() + .map(|id| Self::translate_imported_node_id(*id, imported_ids, external_ids)) + .collect_vec() } - pub fn add_node(&mut self, node: GraphNode) -> GraphNodeId { - let id = self.allocate_node_id(); - self.nodes.insert_with_id(id, node); - id + pub(crate) fn append_node_entries_with_mapping( + &mut self, + nodes: impl IntoIterator, + external_ids: &HashMap, + ) -> HashMap { + let nodes = nodes.into_iter().collect_vec(); + let mut imported_ids = HashMap::new(); + + for entry in nodes { + let id = self.add_node(entry.node); + imported_ids.insert(entry.id, id); + } + + let new_ids = imported_ids.values().copied().collect_vec(); + for id in new_ids { + if let Some(mut node) = self.find_node_by_id_mut(id) { + for input in &mut node.inputs { + *input = Self::translate_imported_node_id(*input, &imported_ids, external_ids); + } + for output in &mut node.outputs { + *output = + Self::translate_imported_node_id(*output, &imported_ids, external_ids); + } + } + } + + imported_ids } pub fn find_node_by_input_name(&mut self, input_name: &str) -> Option> { @@ -766,24 +811,6 @@ impl Graph { Some(self.nodes.remove(index)) } - pub fn rebuild_node_id_base(&mut self, base_index: usize) { - let nodes = std::mem::take(&mut self.nodes) - .into_iter() - .map(|entry| { - let mut node = entry.node; - for index in itertools::chain!(&mut node.inputs, &mut node.outputs) { - *index += base_index; - } - (entry.id + base_index, node) - }) - .collect_vec(); - self.nodes = GraphNodes::from(nodes); - self.next_node_id = self.next_node_id(); - - self.build_producers(); - self.build_consumers(); - } - pub fn remove_by_node_id_lazy(&mut self, node_id: GraphNodeId) { let Some(index) = self.nodes.iter().position(|node| node.id == node_id) else { return; @@ -800,7 +827,7 @@ impl Graph { outputs: Vec, tag: String, ) -> GraphNodeId { - let replacement_id = self.allocate_node_id(); + let replacement_id = self.nodes.allocate_node_id(); for input in &inputs { if let Some(mut node) = self.find_node_by_id_mut(*input) { @@ -1232,7 +1259,8 @@ impl From<&SubGraphWithGraph<'_>> for Graph { } let mut graph = Graph::from_nodes_with_ids(nodes); - graph.rebuild_node_id_base(0); + graph.build_producers(); + graph.build_consumers(); graph } @@ -1533,4 +1561,77 @@ mod tests { assert_eq!(id, 31); assert!(graph.find_node_by_id(31).is_some()); } + + #[test] + fn concat_allocates_fresh_ids_and_remaps_imported_edges() { + let mut graph = Graph::from_nodes_with_ids(vec![( + 10, + GraphNode { + kind: GraphNodeKind::Input("a".to_owned()), + ..Default::default() + }, + )]); + let other = Graph::from_nodes_with_ids(vec![ + ( + 100, + GraphNode { + outputs: vec![101], + ..Default::default() + }, + ), + ( + 101, + GraphNode { + inputs: vec![100], + ..Default::default() + }, + ), + ]); + + graph.concat(other); + + assert_eq!( + graph.nodes.iter().map(|node| node.id).collect_vec(), + vec![10, 11, 12] + ); + assert_eq!(graph.find_node_by_id(11).unwrap().outputs, vec![12]); + assert_eq!(graph.find_node_by_id(12).unwrap().inputs, vec![11]); + } + + #[test] + fn merge_by_input_allocates_fresh_ids_and_reuses_shared_input() { + let mut graph = Graph::from_nodes_with_ids(vec![( + 10, + GraphNode { + kind: GraphNodeKind::Input("a".to_owned()), + ..Default::default() + }, + )]); + let other = Graph::from_nodes_with_ids(vec![ + ( + 100, + GraphNode { + kind: GraphNodeKind::Input("a".to_owned()), + outputs: vec![101], + ..Default::default() + }, + ), + ( + 101, + GraphNode { + inputs: vec![100], + ..Default::default() + }, + ), + ]); + + graph.merge_by_input(other); + + assert_eq!( + graph.nodes.iter().map(|node| node.id).collect_vec(), + vec![10, 11] + ); + assert_eq!(graph.find_node_by_id(10).unwrap().outputs, vec![11]); + assert_eq!(graph.find_node_by_id(11).unwrap().inputs, vec![10]); + } } diff --git a/src/transform/logic.rs b/src/transform/logic.rs index f003b76..3a3ce94 100644 --- a/src/transform/logic.rs +++ b/src/transform/logic.rs @@ -209,8 +209,6 @@ impl LogicGraphTransformer { // replacable only (x, y) => (z) fn replace_binops_lazy(&mut self, src: GraphNodeId, mut tar: Graph) -> eyre::Result<()> { - tar.rebuild_node_id_base(self.graph.graph.max_node_id().unwrap() + 1); - let g = &mut self.graph.graph; let src_inputs = g.find_node_by_id(src).unwrap().inputs.clone(); @@ -224,26 +222,33 @@ impl LogicGraphTransformer { .collect_vec(); let tar_output = tar.outputs()[0]; let tar_output_input = tar.find_node_by_id(tar.outputs()[0]).unwrap().inputs[0]; + let external_ids = HashMap::from([ + (tar_inputs[0], src_inputs[0]), + (tar_inputs[1], src_inputs[1]), + ]); + + tar.remove_by_node_id_lazy(tar_inputs[0]); + tar.remove_by_node_id_lazy(tar_inputs[1]); + tar.remove_by_node_id_lazy(tar_output); + let imported_ids = g.append_node_entries_with_mapping(tar.nodes, &external_ids); for index in 0..=1 { if let Some(mut node) = g.find_node_by_id_mut(src_inputs[index]) { - node.outputs.extend(tar_inputs_outputs[index].clone()); + node.outputs.extend(Graph::translate_imported_node_ids( + &tar_inputs_outputs[index], + &imported_ids, + &external_ids, + )); node.outputs.retain(|node_id| *node_id != src); } } - tar.replace_node_id_lazy(tar_inputs[0], src_inputs[0]); - tar.replace_node_id_lazy(tar_inputs[1], src_inputs[1]); + let tar_output_input = + Graph::translate_imported_node_id(tar_output_input, &imported_ids, &external_ids); g.replace_input_node_id_lazy(src, tar_output_input); g.remove_by_node_id_lazy(src); - - tar.find_node_by_id_mut(tar_output_input).unwrap().outputs = src_outputs.clone(); - tar.remove_by_node_id_lazy(tar_inputs[0]); - tar.remove_by_node_id_lazy(tar_inputs[1]); - tar.remove_by_node_id_lazy(tar_output); - - g.nodes.extend(tar.nodes); + g.find_node_by_id_mut(tar_output_input).unwrap().outputs = src_outputs; Ok(()) } From 38eb7edd15002560e27b4ac5b60718ffc1cb1c85 Mon Sep 17 00:00:00 2001 From: rollrat Date: Sun, 31 May 2026 19:59:32 +0900 Subject: [PATCH 11/11] Refine graph import replacement API Replace the low-level node-entry append helper with append_graph_with_replacements so graph imports consume whole graphs, remap through a single old-to-current id map, and keep replacement semantics documented. Also covers the merge_by_outin output-name collision regression. --- src/graph/mod.rs | 189 +++++++++++++++++++++++++---------------- src/transform/logic.rs | 22 ++--- 2 files changed, 126 insertions(+), 85 deletions(-) diff --git a/src/graph/mod.rs b/src/graph/mod.rs index da80108..18c45d4 100644 --- a/src/graph/mod.rs +++ b/src/graph/mod.rs @@ -492,7 +492,7 @@ impl Graph { } pub fn concat(&mut self, other: Self) { - self.append_node_entries_with_mapping(other.nodes, &HashMap::new()); + self.append_graph_with_replacements(other, &HashMap::new()); self.build_producers(); self.build_consumers(); } @@ -520,21 +520,18 @@ impl Graph { _ => None, }) .collect(); - let external_ids = shared_inputs + let old_to_existing_ids = shared_inputs .iter() .map(|(from, to, _)| (*from, *to)) .collect::>(); - let imported_ids = self.append_node_entries_with_mapping( - other - .nodes - .into_iter() - .filter(|node| !external_ids.contains_key(&node.id)), - &external_ids, - ); + let old_to_current_ids = self.append_graph_with_replacements(other, &old_to_existing_ids); for (_, to, outputs) in shared_inputs { - let outputs = Self::translate_imported_node_ids(&outputs, &imported_ids, &external_ids); + let outputs = outputs + .iter() + .map(|id| old_to_current_ids.get(id).copied().unwrap_or(*id)) + .collect_vec(); self.find_node_by_id_mut(to) .unwrap() .outputs @@ -572,29 +569,26 @@ impl Graph { _ => None, }) .collect(); - let external_ids = out_in + let old_to_existing_ids = out_in .iter() - .map(|(output, input)| (target_inputs[*input].0, src_outputs[*output])) + .map(|(output, input)| { + let (target_input_id, _) = &target_inputs[*input]; + (*target_input_id, src_outputs[*output]) + }) .collect::>(); - let imported_ids = self.append_node_entries_with_mapping( - target - .nodes - .into_iter() - .filter(|node| !external_ids.contains_key(&node.id)), - &external_ids, - ); - self.nodes.retain(|node| !matches!(&node.kind, GraphNodeKind::Output(name) if src_outputs.contains_key(name.as_str()))); + let old_to_current_ids = self.append_graph_with_replacements(target, &old_to_existing_ids); + for (output, input) in out_in { let out_node = src_outputs[output]; - let in_node = Self::translate_imported_node_ids( - &target_inputs[input].1, - &imported_ids, - &external_ids, - ); + let (_, target_input_outputs) = &target_inputs[input]; + let in_node = target_input_outputs + .iter() + .map(|id| old_to_current_ids.get(id).copied().unwrap_or(*id)) + .collect_vec(); self.find_node_by_id_mut(out_node).unwrap().outputs = in_node; } @@ -658,29 +652,24 @@ impl Graph { }) .collect(); - let mut external_ids = HashMap::new(); + let mut old_to_existing_ids = HashMap::new(); for name in &targets { + let (target_input_id, _) = &target_inputs[name]; if let Some(src_in) = src_inputs.get(name) { - external_ids.insert(target_inputs[name].0, *src_in); + old_to_existing_ids.insert(*target_input_id, *src_in); } else if let Some(src_out) = src_outputs.get(name) { - external_ids.insert(target_inputs[name].0, *src_out); + old_to_existing_ids.insert(*target_input_id, *src_out); } } - let imported_ids = self.append_node_entries_with_mapping( - target - .nodes - .into_iter() - .filter(|node| !external_ids.contains_key(&node.id)), - &external_ids, - ); + let old_to_current_ids = self.append_graph_with_replacements(target, &old_to_existing_ids); for name in &targets { - let tar_in = Self::translate_imported_node_ids( - &target_inputs[name].1, - &imported_ids, - &external_ids, - ); + let (_, target_input_outputs) = &target_inputs[name]; + let tar_in = target_input_outputs + .iter() + .map(|id| old_to_current_ids.get(id).copied().unwrap_or(*id)) + .collect_vec(); if let Some(src_in) = src_inputs.get(name) { self.find_node_by_id_mut(*src_in) @@ -734,56 +723,39 @@ impl Graph { self.nodes.add(node) } - pub(crate) fn translate_imported_node_id( - node_id: GraphNodeId, - imported_ids: &HashMap, - external_ids: &HashMap, - ) -> GraphNodeId { - external_ids - .get(&node_id) - .or_else(|| imported_ids.get(&node_id)) - .copied() - .unwrap_or(node_id) - } - - pub(crate) fn translate_imported_node_ids( - node_ids: &[GraphNodeId], - imported_ids: &HashMap, - external_ids: &HashMap, - ) -> Vec { - node_ids - .iter() - .map(|id| Self::translate_imported_node_id(*id, imported_ids, external_ids)) - .collect_vec() - } - - pub(crate) fn append_node_entries_with_mapping( + /// Appends `imported_graph` into this graph using fresh ids for every imported node. + /// Old ids in `old_to_existing_ids` are not inserted or overwritten; references to + /// those imported nodes are resolved to the existing node ids in this graph. + pub(crate) fn append_graph_with_replacements( &mut self, - nodes: impl IntoIterator, - external_ids: &HashMap, + imported_graph: Self, + old_to_existing_ids: &HashMap, ) -> HashMap { - let nodes = nodes.into_iter().collect_vec(); - let mut imported_ids = HashMap::new(); + let nodes = imported_graph.nodes.into_iter().collect_vec(); + let mut old_to_current_ids = old_to_existing_ids.clone(); + let mut new_ids = Vec::new(); for entry in nodes { + if old_to_existing_ids.contains_key(&entry.id) { + continue; + } let id = self.add_node(entry.node); - imported_ids.insert(entry.id, id); + old_to_current_ids.insert(entry.id, id); + new_ids.push(id); } - let new_ids = imported_ids.values().copied().collect_vec(); for id in new_ids { if let Some(mut node) = self.find_node_by_id_mut(id) { for input in &mut node.inputs { - *input = Self::translate_imported_node_id(*input, &imported_ids, external_ids); + *input = old_to_current_ids.get(input).copied().unwrap_or(*input); } for output in &mut node.outputs { - *output = - Self::translate_imported_node_id(*output, &imported_ids, external_ids); + *output = old_to_current_ids.get(output).copied().unwrap_or(*output); } } } - imported_ids + old_to_current_ids } pub fn find_node_by_input_name(&mut self, input_name: &str) -> Option> { @@ -1634,4 +1606,73 @@ mod tests { assert_eq!(graph.find_node_by_id(10).unwrap().outputs, vec![11]); assert_eq!(graph.find_node_by_id(11).unwrap().inputs, vec![10]); } + + #[test] + fn merge_by_outin_preserves_imported_output_with_same_name_as_removed_output() { + let mut graph = Graph::from_nodes_with_ids(vec![ + ( + 10, + GraphNode { + outputs: vec![11], + ..Default::default() + }, + ), + ( + 11, + GraphNode { + inputs: vec![10], + outputs: vec![12], + ..Default::default() + }, + ), + ( + 12, + GraphNode { + kind: GraphNodeKind::Output("z".to_owned()), + inputs: vec![11], + ..Default::default() + }, + ), + ]); + let target = Graph::from_nodes_with_ids(vec![ + ( + 100, + GraphNode { + kind: GraphNodeKind::Input("i".to_owned()), + outputs: vec![101], + ..Default::default() + }, + ), + ( + 101, + GraphNode { + inputs: vec![100], + outputs: vec![102], + ..Default::default() + }, + ), + ( + 102, + GraphNode { + kind: GraphNodeKind::Output("z".to_owned()), + inputs: vec![101], + ..Default::default() + }, + ), + ]); + + graph.merge_by_outin(target, vec![("z", "i")]); + + let outputs = graph + .nodes + .iter() + .filter(|node| matches!(&node.kind, GraphNodeKind::Output(name) if name == "z")) + .map(|node| node.id) + .collect_vec(); + assert_eq!(outputs, vec![14]); + assert_eq!(graph.find_node_by_id(11).unwrap().outputs, vec![13]); + assert_eq!(graph.find_node_by_id(13).unwrap().inputs, vec![11]); + assert_eq!(graph.find_node_by_id(13).unwrap().outputs, vec![14]); + assert_eq!(graph.find_node_by_id(14).unwrap().inputs, vec![13]); + } } diff --git a/src/transform/logic.rs b/src/transform/logic.rs index 3a3ce94..fa89f74 100644 --- a/src/transform/logic.rs +++ b/src/transform/logic.rs @@ -222,29 +222,29 @@ impl LogicGraphTransformer { .collect_vec(); let tar_output = tar.outputs()[0]; let tar_output_input = tar.find_node_by_id(tar.outputs()[0]).unwrap().inputs[0]; - let external_ids = HashMap::from([ + let old_to_existing_ids = HashMap::from([ (tar_inputs[0], src_inputs[0]), (tar_inputs[1], src_inputs[1]), ]); - tar.remove_by_node_id_lazy(tar_inputs[0]); - tar.remove_by_node_id_lazy(tar_inputs[1]); tar.remove_by_node_id_lazy(tar_output); - let imported_ids = g.append_node_entries_with_mapping(tar.nodes, &external_ids); + let old_to_current_ids = g.append_graph_with_replacements(tar, &old_to_existing_ids); for index in 0..=1 { if let Some(mut node) = g.find_node_by_id_mut(src_inputs[index]) { - node.outputs.extend(Graph::translate_imported_node_ids( - &tar_inputs_outputs[index], - &imported_ids, - &external_ids, - )); + node.outputs.extend( + tar_inputs_outputs[index] + .iter() + .map(|id| old_to_current_ids.get(id).copied().unwrap_or(*id)), + ); node.outputs.retain(|node_id| *node_id != src); } } - let tar_output_input = - Graph::translate_imported_node_id(tar_output_input, &imported_ids, &external_ids); + let tar_output_input = old_to_current_ids + .get(&tar_output_input) + .copied() + .unwrap_or(tar_output_input); g.replace_input_node_id_lazy(src, tar_output_input); g.remove_by_node_id_lazy(src);