From 1759d23160f3c30dd9fa48907fb106b65f79ecba Mon Sep 17 00:00:00 2001 From: Luca Mondada Date: Mon, 11 Sep 2023 14:55:55 +0200 Subject: [PATCH] feat!: SiblingSubgraph does not borrow Hugr --- src/hugr/views/sibling_subgraph.rs | 184 ++++++++++------------------- 1 file changed, 61 insertions(+), 123 deletions(-) diff --git a/src/hugr/views/sibling_subgraph.rs b/src/hugr/views/sibling_subgraph.rs index c344be6b24..989a64430f 100644 --- a/src/hugr/views/sibling_subgraph.rs +++ b/src/hugr/views/sibling_subgraph.rs @@ -42,15 +42,18 @@ use super::HugrView; /// /// The ordering of the nodes in the subgraph is irrelevant to define the convex /// subgraph, but it determines the ordering of the boundary signature. +/// +/// No reference to the underlying graph is kept. Thus most of the subgraph +/// methods expect a reference to the Hugr as an argument. /// /// At the moment we do not support state order edges at the subgraph boundary. /// The `boundary_port` and `signature` methods will panic if any are found. /// State order edges are also unsupported in replacements in /// `create_simple_replacement`. +// TODO: implement a borrowing wrapper that implements a view into the Hugr +// given a reference. #[derive(Clone, Debug)] -pub struct SiblingSubgraph<'g, Base> { - /// The underlying Hugr. - base: &'g Base, +pub struct SiblingSubgraph { /// The nodes of the induced subgraph. nodes: Vec, /// The input ports of the subgraph. @@ -70,18 +73,18 @@ pub type IncomingPorts = Vec>; /// The type of the outgoing boundary of [`SiblingSubgraph`]. pub type OutgoingPorts = Vec<(Node, Port)>; -impl<'g, Base: HugrView> SiblingSubgraph<'g, Base> { +impl SiblingSubgraph { /// A sibling subgraph from a [`crate::ops::OpTag::DataflowParent`]-rooted HUGR. /// /// The subgraph is given by the nodes between the input and output - /// children nodes of the parent node. If you wish to create a subgraph + /// children nodes of the root node. If you wish to create a subgraph /// from another root, wrap the `region` argument in a [`super::SiblingGraph`]. /// /// This will return an [`InvalidSubgraph::EmptySubgraph`] error if the /// subgraph is empty. - pub fn try_from_dataflow_graph(dfg_graph: &'g Base) -> Result + pub fn try_new_dataflow_subgraph(dfg_graph: &H) -> Result where - Base: Clone + HugrView, + H: Clone + HugrView, Root: ContainerHandle, { let parent = dfg_graph.root(); @@ -94,7 +97,6 @@ impl<'g, Base: HugrView> SiblingSubgraph<'g, Base> { Err(InvalidSubgraph::EmptySubgraph) } else { Ok(Self { - base: dfg_graph, nodes, inputs, outputs, @@ -102,7 +104,7 @@ impl<'g, Base: HugrView> SiblingSubgraph<'g, Base> { } } - /// Create a new sibling subgraph from some boundary edges. + /// Create a new convex sibling subgraph from input and output boundaries. /// /// Any sibling subgraph can be defined using two sets of boundary edges /// $B_I$ and $B_O$, the incoming and outgoing boundary edges respectively. @@ -141,36 +143,31 @@ impl<'g, Base: HugrView> SiblingSubgraph<'g, Base> { /// /// This function fails if the subgraph is not convex, if the nodes /// do not share a common parent or if the subgraph is empty. - pub fn try_from_boundary_ports( - base: &'g Base, + pub fn try_new( incoming: IncomingPorts, outgoing: OutgoingPorts, - ) -> Result - where - Base: Clone + HugrView, - { - let mut checker = ConvexChecker::new(base); - Self::try_from_boundary_ports_with_checker(base, incoming, outgoing, &mut checker) + hugr: &impl HugrView, + ) -> Result { + let mut checker = ConvexChecker::new(hugr); + Self::try_new_with_checker(incoming, outgoing, hugr, &mut checker) } - /// Create a new sibling subgraph from some boundary edges. + /// Create a new convex sibling subgraph from input and output boundaries. /// /// Provide a [`ConvexChecker`] instance to avoid constructing one for /// faster convexity check. If you do not have one, use - /// [`SiblingSubgraph::try_from_boundary_ports`]. + /// [`SiblingSubgraph::try_new`]. /// - /// Refer to [`SiblingSubgraph::try_from_boundary_ports`] for the full + /// Refer to [`SiblingSubgraph::try_new`] for the full /// documentation. - pub fn try_from_boundary_ports_with_checker( - base: &'g Base, + pub fn try_new_with_checker<'c, 'h: 'c, H: HugrView>( inputs: IncomingPorts, outputs: OutgoingPorts, - checker: &mut ConvexChecker<'g, Base>, - ) -> Result - where - Base: Clone + HugrView, - { - let pg = base.portgraph(); + hugr: &'h H, + checker: &'c mut ConvexChecker<'h, H>, + ) -> Result { + let pg = hugr.portgraph(); + let to_pg = |(n, p): (Node, Port)| pg.port_index(n.index, p.offset).expect("invalid port"); // Ordering of the edges here is preserved and becomes ordering of the signature. @@ -184,63 +181,13 @@ impl<'g, Base: HugrView> SiblingSubgraph<'g, Base> { .map(to_pg), ); let nodes = subpg.nodes_iter().map_into().collect_vec(); - - validate_subgraph(base, &nodes, &inputs, &outputs)?; + validate_subgraph(hugr, &nodes, &inputs, &outputs)?; if !subpg.is_convex_with_checker(&mut checker.0) { return Err(InvalidSubgraph::NotConvex); } Ok(Self { - base, - nodes, - inputs, - outputs, - }) - } - - /// Create a new convex sibling subgraph from a set of nodes. - /// - /// This fails if the set of nodes is not convex, nodes do not share a - /// common parent or the subgraph is empty. - pub fn try_new( - base: &'g Base, - nodes: Vec, - inputs: IncomingPorts, - outputs: OutgoingPorts, - ) -> Result - where - Base: HugrView, - { - let mut checker = ConvexChecker::new(base); - Self::try_new_with_checker(base, nodes, inputs, outputs, &mut checker) - } - - /// Create a new convex sibling subgraph from a set of nodes. - /// - /// Provide a [`ConvexChecker`] instance to avoid constructing one for - /// faster convexity check. If you do not have one, use [`SiblingSubgraph::try_new`]. - /// - /// This fails if the set of nodes is not convex, nodes do not share a - /// common parent or the subgraph is empty. - pub fn try_new_with_checker( - base: &'g Base, - nodes: Vec, - inputs: IncomingPorts, - outputs: OutgoingPorts, - checker: &mut ConvexChecker<'g, Base>, - ) -> Result - where - Base: HugrView, - { - validate_subgraph(base, &nodes, &inputs, &outputs)?; - - if !checker.0.is_node_convex(nodes.iter().map(|n| n.index)) { - return Err(InvalidSubgraph::NotConvex); - } - - Ok(Self { - base, nodes, inputs, outputs, @@ -253,16 +200,13 @@ impl<'g, Base: HugrView> SiblingSubgraph<'g, Base> { } /// The signature of the subgraph. - pub fn signature(&self) -> FunctionType - where - Base: HugrView, - { + pub fn signature(&self, hugr: &impl HugrView) -> FunctionType { let input = self .inputs .iter() .map(|part| { let &(n, p) = part.iter().next().expect("is non-empty"); - let sig = self.base.get_optype(n).signature(); + let sig = hugr.get_optype(n).signature(); sig.get(p).cloned().expect("must be dataflow edge") }) .collect_vec(); @@ -270,7 +214,7 @@ impl<'g, Base: HugrView> SiblingSubgraph<'g, Base> { .outputs .iter() .map(|&(n, p)| { - let sig = self.base.get_optype(n).signature(); + let sig = hugr.get_optype(n).signature(); sig.get(p).cloned().expect("must be dataflow edge") }) .collect_vec(); @@ -278,13 +222,8 @@ impl<'g, Base: HugrView> SiblingSubgraph<'g, Base> { } /// The parent of the sibling subgraph. - pub fn get_parent(&self) -> Node - where - Base: HugrView, - { - self.base - .get_parent(self.nodes[0]) - .expect("invalid subgraph") + pub fn get_parent(&self, hugr: &impl HugrView) -> Node { + hugr.get_parent(self.nodes[0]).expect("invalid subgraph") } /// Construct a [`SimpleReplacement`] to replace `self` with `replacement`. @@ -306,11 +245,9 @@ impl<'g, Base: HugrView> SiblingSubgraph<'g, Base> { /// the replacement graph, this will panic. pub fn create_simple_replacement( &self, + hugr: &impl HugrView, replacement: Hugr, - ) -> Result - where - Base: HugrView, - { + ) -> Result { let removal = self.nodes().iter().copied().collect(); let rep_root = replacement.root(); @@ -322,7 +259,7 @@ impl<'g, Base: HugrView> SiblingSubgraph<'g, Base> { else { return Err(InvalidReplacement::InvalidDataflowParent); }; - if dfg_optype.signature() != self.signature() { + if dfg_optype.signature() != self.signature(hugr) { return Err(InvalidReplacement::InvalidSignature); } @@ -357,14 +294,13 @@ impl<'g, Base: HugrView> SiblingSubgraph<'g, Base> { .iter() .zip_eq(rep_outputs) .flat_map(|(&(self_source_n, self_source_p), (_, rep_target_p))| { - self.base - .linked_ports(self_source_n, self_source_p) + hugr.linked_ports(self_source_n, self_source_p) .map(move |self_target| (self_target, rep_target_p)) }) .collect(); Ok(SimpleReplacement::new( - self.get_parent(), + self.get_parent(hugr), removal, replacement, nu_inp, @@ -580,7 +516,7 @@ mod tests { use super::*; - impl<'g, Base: HugrView> SiblingSubgraph<'g, Base> { + impl SiblingSubgraph { /// A sibling subgraph from a HUGR. /// /// The subgraph is given by the sibling graph of the root. If you wish to @@ -589,17 +525,13 @@ mod tests { /// /// This will return an [`InvalidSubgraph::EmptySubgraph`] error if the /// subgraph is empty. - fn from_sibling_graph(sibling_graph: &'g Base) -> Result - where - Base: HugrView, - { + fn from_sibling_graph(sibling_graph: &impl HugrView) -> Result { let root = sibling_graph.root(); let nodes = sibling_graph.children(root).collect_vec(); if nodes.is_empty() { Err(InvalidSubgraph::EmptySubgraph) } else { Ok(Self { - base: sibling_graph, nodes, inputs: Vec::new(), outputs: Vec::new(), @@ -651,8 +583,14 @@ mod tests { let from_root = SiblingSubgraph::from_sibling_graph(&sibling_graph)?; let region: SiblingGraph<'_> = SiblingGraph::new(&hugr, func_root); let from_region = SiblingSubgraph::from_sibling_graph(®ion)?; - assert_eq!(from_root.get_parent(), from_region.get_parent()); - assert_eq!(from_root.signature(), from_region.signature()); + assert_eq!( + from_root.get_parent(&sibling_graph), + from_region.get_parent(&sibling_graph) + ); + assert_eq!( + from_root.signature(&sibling_graph), + from_region.signature(&sibling_graph) + ); Ok(()) } @@ -660,7 +598,7 @@ mod tests { fn construct_simple_replacement() -> Result<(), InvalidSubgraph> { let (mut hugr, func_root) = build_hugr().unwrap(); let func: SiblingGraph<'_, FuncID> = SiblingGraph::new(&hugr, func_root); - let sub = SiblingSubgraph::try_from_dataflow_graph(&func)?; + let sub = SiblingSubgraph::try_new_dataflow_subgraph(&func)?; let empty_dfg = { let builder = DFGBuilder::new(FunctionType::new_linear(type_row![QB_T, QB_T])).unwrap(); @@ -668,7 +606,7 @@ mod tests { builder.finish_prelude_hugr_with_outputs(inputs).unwrap() }; - let rep = sub.create_simple_replacement(empty_dfg).unwrap(); + let rep = sub.create_simple_replacement(&func, empty_dfg).unwrap(); assert_eq!(rep.removal.len(), 1); @@ -683,9 +621,9 @@ mod tests { fn test_signature() -> Result<(), InvalidSubgraph> { let (hugr, dfg) = build_hugr().unwrap(); let func: SiblingGraph<'_, FuncID> = SiblingGraph::new(&hugr, dfg); - let sub = SiblingSubgraph::try_from_dataflow_graph(&func)?; + let sub = SiblingSubgraph::try_new_dataflow_subgraph(&func)?; assert_eq!( - sub.signature(), + sub.signature(&func), FunctionType::new_linear(type_row![QB_T, QB_T]) ); Ok(()) @@ -704,7 +642,7 @@ mod tests { }; assert_eq!( - sub.create_simple_replacement(empty_dfg).unwrap_err(), + sub.create_simple_replacement(&func, empty_dfg).unwrap_err(), InvalidReplacement::InvalidSignature ); Ok(()) @@ -715,7 +653,7 @@ mod tests { let (hugr, func_root) = build_hugr().unwrap(); let func: SiblingGraph<'_, FuncID> = SiblingGraph::new(&hugr, func_root); assert_eq!( - SiblingSubgraph::try_from_dataflow_graph(&func) + SiblingSubgraph::try_new_dataflow_subgraph(&func) .unwrap() .nodes() .len(), @@ -729,8 +667,7 @@ mod tests { let (inp, out) = hugr.children(func_root).take(2).collect_tuple().unwrap(); let func: SiblingGraph<'_> = SiblingGraph::new(&hugr, func_root); // All graph except input/output nodes - SiblingSubgraph::try_from_boundary_ports( - &func, + SiblingSubgraph::try_new( hugr.node_outputs(inp) .map(|p| hugr.linked_ports(inp, p).collect_vec()) .filter(|ps| !ps.is_empty()) @@ -738,6 +675,7 @@ mod tests { hugr.node_inputs(out) .filter_map(|p| hugr.linked_ports(out, p).exactly_one().ok()) .collect(), + &func, ) .unwrap(); } @@ -750,10 +688,10 @@ mod tests { let first_cx_edge = hugr.node_outputs(inp).next().unwrap(); // All graph but one edge assert!(matches!( - SiblingSubgraph::try_from_boundary_ports( - &func, + SiblingSubgraph::try_new( vec![hugr.linked_ports(inp, first_cx_edge).collect()], vec![(inp, first_cx_edge)], + &func, ), Err(InvalidSubgraph::NotConvex) )); @@ -768,10 +706,10 @@ mod tests { let snd_cx_edge = hugr.node_inputs(out).next().unwrap(); // All graph but one edge assert!(matches!( - SiblingSubgraph::try_from_boundary_ports( - &func, + SiblingSubgraph::try_new( vec![vec![(out, snd_cx_edge)]], vec![(inp, first_cx_edge)], + &func, ), Err(InvalidSubgraph::NotConvex) )); @@ -780,11 +718,11 @@ mod tests { #[test] fn preserve_signature() { let (hugr, func_root) = build_hugr_classical().unwrap(); - let func: SiblingGraph<'_, FuncID> = SiblingGraph::new(&hugr, func_root); - let func = SiblingSubgraph::try_from_dataflow_graph(&func).unwrap(); + let func_graph: SiblingGraph<'_, FuncID> = SiblingGraph::new(&hugr, func_root); + let func = SiblingSubgraph::try_new_dataflow_subgraph(&func_graph).unwrap(); let OpType::FuncDefn(func_defn) = hugr.get_optype(func_root) else { panic!() }; - assert_eq!(func_defn.signature, func.signature()) + assert_eq!(func_defn.signature, func.signature(&func_graph)) } }