From 651fd5d01a16ec84caa1700b2c9ccc23f6cdd71b Mon Sep 17 00:00:00 2001 From: Alec Edgington Date: Wed, 30 Aug 2023 15:56:30 +0100 Subject: [PATCH 1/9] WIP on InsertIdentity. --- src/hugr/rewrite.rs | 1 + src/hugr/rewrite/insert_identity.rs | 62 +++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100644 src/hugr/rewrite/insert_identity.rs diff --git a/src/hugr/rewrite.rs b/src/hugr/rewrite.rs index 415c39a18..e6b7a2031 100644 --- a/src/hugr/rewrite.rs +++ b/src/hugr/rewrite.rs @@ -1,5 +1,6 @@ //! Rewrite operations on the HUGR - replacement, outlining, etc. +pub mod insert_identity; pub mod outline_cfg; pub mod simple_replace; use std::mem; diff --git a/src/hugr/rewrite/insert_identity.rs b/src/hugr/rewrite/insert_identity.rs new file mode 100644 index 000000000..61be774a4 --- /dev/null +++ b/src/hugr/rewrite/insert_identity.rs @@ -0,0 +1,62 @@ +//! Implementation of the `InsertIdentity` operation. + +use crate::hugr::{HugrMut, Node}; +use crate::{Hugr, HugrView, Port}; + +use super::Rewrite; + +use itertools::Itertools; +use thiserror::Error; + +/// Specification of a identity-insertion operation. +#[derive(Debug, Clone)] +pub struct IdentityInsertion { + /// The node following the identity to be inserted. + pub post_node: Node, + /// The port following the identity to be inserted. + pub post_port: Port, +} + +impl IdentityInsertion { + /// Create a new [`IdentityInsertion`] specification. + pub fn new(post_node: Node, post_port: Port) -> Self { + Self { + post_node, + post_port, + } + } +} + +/// Error from an [`IdentityInsertion`] operation. +#[derive(Debug, Clone, Error, PartialEq, Eq)] +pub enum IdentityInsertionError { + /// Invalid node. + #[error("Node is invalid.")] + InvalidNode(), + /// Invalid port. + #[error("post_port is invalid.")] + InvalidPort(), +} + +impl Rewrite for IdentityInsertion { + type Error = IdentityInsertionError; + const UNCHANGED_ON_FAILURE: bool = true; + fn verify(&self, _h: &Hugr) -> Result<(), IdentityInsertionError> { + unimplemented!() + } + fn apply(self, h: &mut Hugr) -> Result<(), IdentityInsertionError> { + let (pre_node, pre_port) = h + .linked_ports(self.post_node, self.post_port) + .exactly_one() + .unwrap(); + h.disconnect(self.post_node, self.post_port).unwrap(); + h.connect( + pre_node, + pre_port.index(), + self.post_node, + self.post_port.index(), + ) + .unwrap(); + Ok(()) + } +} From c29e707a110034906714ddd972966fbd5dde5a41 Mon Sep 17 00:00:00 2001 From: Alec Edgington Date: Wed, 30 Aug 2023 17:19:57 +0100 Subject: [PATCH 2/9] TODO comment. --- src/hugr/rewrite/insert_identity.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/hugr/rewrite/insert_identity.rs b/src/hugr/rewrite/insert_identity.rs index 61be774a4..037e98bdc 100644 --- a/src/hugr/rewrite/insert_identity.rs +++ b/src/hugr/rewrite/insert_identity.rs @@ -50,6 +50,7 @@ impl Rewrite for IdentityInsertion { .exactly_one() .unwrap(); h.disconnect(self.post_node, self.post_port).unwrap(); + // TODO Check type, insert Noop... h.connect( pre_node, pre_port.index(), From c11beb2d61fad7586a720fef95b308e9c1e4f0d2 Mon Sep 17 00:00:00 2001 From: Seyon Sivarajah Date: Thu, 31 Aug 2023 10:07:58 +0100 Subject: [PATCH 3/9] add some assumption/condition comments --- src/hugr/rewrite/insert_identity.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/hugr/rewrite/insert_identity.rs b/src/hugr/rewrite/insert_identity.rs index 037e98bdc..339bb52e7 100644 --- a/src/hugr/rewrite/insert_identity.rs +++ b/src/hugr/rewrite/insert_identity.rs @@ -42,13 +42,21 @@ impl Rewrite for IdentityInsertion { type Error = IdentityInsertionError; const UNCHANGED_ON_FAILURE: bool = true; fn verify(&self, _h: &Hugr) -> Result<(), IdentityInsertionError> { + /* + Assumptions: + 1. Value kind inputs can only have one connection. + Conditions: + 1. post_port is Value kind + 2. post_port is connected to a sibling of post_node + */ + unimplemented!() } fn apply(self, h: &mut Hugr) -> Result<(), IdentityInsertionError> { let (pre_node, pre_port) = h .linked_ports(self.post_node, self.post_port) .exactly_one() - .unwrap(); + .expect("Value kind input can only have one connection."); h.disconnect(self.post_node, self.post_port).unwrap(); // TODO Check type, insert Noop... h.connect( From c4e71cdcb2f02631d77979b60991d0d02c5be424 Mon Sep 17 00:00:00 2001 From: Seyon Sivarajah Date: Thu, 31 Aug 2023 10:21:08 +0100 Subject: [PATCH 4/9] use ApplyResult --- src/hugr/rewrite/insert_identity.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/hugr/rewrite/insert_identity.rs b/src/hugr/rewrite/insert_identity.rs index 339bb52e7..3ea236ed9 100644 --- a/src/hugr/rewrite/insert_identity.rs +++ b/src/hugr/rewrite/insert_identity.rs @@ -1,6 +1,8 @@ //! Implementation of the `InsertIdentity` operation. +use crate::extension::prelude::QB_T; use crate::hugr::{HugrMut, Node}; +use crate::ops::LeafOp; use crate::{Hugr, HugrView, Port}; use super::Rewrite; @@ -40,6 +42,8 @@ pub enum IdentityInsertionError { impl Rewrite for IdentityInsertion { type Error = IdentityInsertionError; + /// The inserted node. + type ApplyResult = Node; const UNCHANGED_ON_FAILURE: bool = true; fn verify(&self, _h: &Hugr) -> Result<(), IdentityInsertionError> { /* @@ -52,12 +56,13 @@ impl Rewrite for IdentityInsertion { unimplemented!() } - fn apply(self, h: &mut Hugr) -> Result<(), IdentityInsertionError> { + fn apply(self, h: &mut Hugr) -> Result { let (pre_node, pre_port) = h .linked_ports(self.post_node, self.post_port) .exactly_one() .expect("Value kind input can only have one connection."); h.disconnect(self.post_node, self.post_port).unwrap(); + let new_node = h.add_op(LeafOp::Noop { ty: QB_T }); // TODO Check type, insert Noop... h.connect( pre_node, @@ -66,6 +71,6 @@ impl Rewrite for IdentityInsertion { self.post_port.index(), ) .unwrap(); - Ok(()) + Ok(new_node) } } From bbbd0e021cbf73682aafb54561fd15222fbba91d Mon Sep 17 00:00:00 2001 From: Seyon Sivarajah Date: Thu, 31 Aug 2023 10:43:23 +0100 Subject: [PATCH 5/9] turn simple_replace test samples in to fixtures --- src/hugr/rewrite/simple_replace.rs | 37 +++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/src/hugr/rewrite/simple_replace.rs b/src/hugr/rewrite/simple_replace.rs index 966aa765a..750a09778 100644 --- a/src/hugr/rewrite/simple_replace.rs +++ b/src/hugr/rewrite/simple_replace.rs @@ -190,11 +190,11 @@ pub enum SimpleReplacementError { } #[cfg(test)] -mod test { - use std::collections::{HashMap, HashSet}; - +pub(super) mod test { use itertools::Itertools; use portgraph::Direction; + use rstest::{fixture, rstest}; + use std::collections::{HashMap, HashSet}; use crate::builder::{ BuildError, Container, DFGBuilder, Dataflow, DataflowHugr, DataflowSubContainer, @@ -257,6 +257,10 @@ mod test { Ok(module_builder.finish_hugr()?) } + #[fixture] + fn simple_hugr() -> Hugr { + make_hugr().unwrap() + } /// Creates a hugr with a DFG root like the following: /// ┌───┐ /// ┤ H ├──■── @@ -274,6 +278,11 @@ mod test { dfg_builder.finish_hugr_with_outputs(wire45.outputs()) } + #[fixture] + fn dfg_hugr() -> Hugr { + make_dfg_hugr().unwrap() + } + /// Creates a hugr with a DFG root like the following: /// ───── /// ┌───┐ @@ -289,7 +298,12 @@ mod test { dfg_builder.finish_hugr_with_outputs(wireoutvec) } - #[test] + #[fixture] + fn dfg_hugr2() -> Hugr { + make_dfg_hugr2().unwrap() + } + + #[rstest] /// Replace the /// ┌───┐ /// ──■──┤ H ├ @@ -308,8 +322,8 @@ mod test { /// ├───┤┌─┴─┐ /// ┤ H ├┤ X ├ /// └───┘└───┘ - fn test_simple_replacement() { - let mut h: Hugr = make_hugr().unwrap(); + fn test_simple_replacement(simple_hugr: Hugr, dfg_hugr: Hugr) { + let mut h: Hugr = simple_hugr; // 1. Find the DFG node for the inner circuit let p: Node = h .nodes() @@ -323,7 +337,7 @@ mod test { let (h_node_h0, h_node_h1) = h.output_neighbours(h_node_cx).collect_tuple().unwrap(); let s: HashSet = vec![h_node_cx, h_node_h0, h_node_h1].into_iter().collect(); // 3. Construct a new DFG-rooted hugr for the replacement - let n: Hugr = make_dfg_hugr().unwrap(); + let n: Hugr = dfg_hugr; // 4. Construct the input and output matchings // 4.1. Locate the CX and its predecessor H's in n let n_node_cx = n @@ -376,7 +390,7 @@ mod test { assert_eq!(h.validate(), Ok(())); } - #[test] + #[rstest] /// Replace the /// /// ──■── @@ -394,8 +408,9 @@ mod test { /// ┌───┐ /// ┤ H ├ /// └───┘ - fn test_simple_replacement_with_empty_wires() { - let mut h: Hugr = make_hugr().unwrap(); + fn test_simple_replacement_with_empty_wires(simple_hugr: Hugr, dfg_hugr2: Hugr) { + let mut h: Hugr = simple_hugr; + // 1. Find the DFG node for the inner circuit let p: Node = h .nodes() @@ -408,7 +423,7 @@ mod test { .unwrap(); let s: HashSet = vec![h_node_cx].into_iter().collect(); // 3. Construct a new DFG-rooted hugr for the replacement - let n: Hugr = make_dfg_hugr2().unwrap(); + let n: Hugr = dfg_hugr2; // 4. Construct the input and output matchings // 4.1. Locate the Output and its predecessor H in n let n_node_output = n From 8d976e83ad60498a0699c6b78495dbeb4bc34208 Mon Sep 17 00:00:00 2001 From: Seyon Sivarajah Date: Thu, 31 Aug 2023 10:43:47 +0100 Subject: [PATCH 6/9] implement noop node adding --- src/hugr/rewrite/insert_identity.rs | 47 ++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/src/hugr/rewrite/insert_identity.rs b/src/hugr/rewrite/insert_identity.rs index 3ea236ed9..cb4c26ac1 100644 --- a/src/hugr/rewrite/insert_identity.rs +++ b/src/hugr/rewrite/insert_identity.rs @@ -1,8 +1,8 @@ //! Implementation of the `InsertIdentity` operation. -use crate::extension::prelude::QB_T; use crate::hugr::{HugrMut, Node}; use crate::ops::LeafOp; +use crate::types::EdgeKind; use crate::{Hugr, HugrView, Port}; use super::Rewrite; @@ -35,9 +35,9 @@ pub enum IdentityInsertionError { /// Invalid node. #[error("Node is invalid.")] InvalidNode(), - /// Invalid port. - #[error("post_port is invalid.")] - InvalidPort(), + /// Invalid port kind. + #[error("post_port has invalid kind {0:?}. Must be Value.")] + InvalidPortKind(Option), } impl Rewrite for IdentityInsertion { @@ -49,9 +49,10 @@ impl Rewrite for IdentityInsertion { /* Assumptions: 1. Value kind inputs can only have one connection. + 2. Node exists. Conditions: - 1. post_port is Value kind - 2. post_port is connected to a sibling of post_node + 1. post_port is Value kind. + 2. post_port is connected to a sibling of post_node. */ unimplemented!() @@ -61,16 +62,34 @@ impl Rewrite for IdentityInsertion { .linked_ports(self.post_node, self.post_port) .exactly_one() .expect("Value kind input can only have one connection."); + + let kind = h.get_optype(self.post_node).port_kind(self.post_port); + let Some(EdgeKind::Value(ty)) = kind else { + return Err(IdentityInsertionError::InvalidPortKind(kind)); + }; + h.disconnect(self.post_node, self.post_port).unwrap(); - let new_node = h.add_op(LeafOp::Noop { ty: QB_T }); + let new_node = h.add_op(LeafOp::Noop { ty }); + h.connect(pre_node, pre_port.index(), new_node, 0) + .expect("Should only fail if ports don't exist."); + // TODO Check type, insert Noop... - h.connect( - pre_node, - pre_port.index(), - self.post_node, - self.post_port.index(), - ) - .unwrap(); + h.connect(new_node, 0, self.post_node, self.post_port.index()) + .expect("Should only fail if ports don't exist."); Ok(new_node) } } + +#[cfg(test)] +mod tests { + use rstest::{fixture, rstest}; + + use crate::Hugr; + + #[fixture] + fn sample() -> Hugr { + todo!() + } + #[rstest] + fn correct_insertion() {} +} From 90402647f7792527dde5cf4f081b7d4537070486 Mon Sep 17 00:00:00 2001 From: Seyon Sivarajah Date: Thu, 31 Aug 2023 10:47:00 +0100 Subject: [PATCH 7/9] fixup! turn simple_replace test samples in to fixtures --- src/hugr/rewrite/simple_replace.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/hugr/rewrite/simple_replace.rs b/src/hugr/rewrite/simple_replace.rs index 750a09778..6fe75c291 100644 --- a/src/hugr/rewrite/simple_replace.rs +++ b/src/hugr/rewrite/simple_replace.rs @@ -190,7 +190,7 @@ pub enum SimpleReplacementError { } #[cfg(test)] -pub(super) mod test { +pub(in crate::hugr::rewrite) mod test { use itertools::Itertools; use portgraph::Direction; use rstest::{fixture, rstest}; @@ -258,7 +258,7 @@ pub(super) mod test { } #[fixture] - fn simple_hugr() -> Hugr { + pub(in crate::hugr::rewrite) fn simple_hugr() -> Hugr { make_hugr().unwrap() } /// Creates a hugr with a DFG root like the following: @@ -279,7 +279,7 @@ pub(super) mod test { } #[fixture] - fn dfg_hugr() -> Hugr { + pub(in crate::hugr::rewrite) fn dfg_hugr() -> Hugr { make_dfg_hugr().unwrap() } @@ -299,7 +299,7 @@ pub(super) mod test { } #[fixture] - fn dfg_hugr2() -> Hugr { + pub(in crate::hugr::rewrite) fn dfg_hugr2() -> Hugr { make_dfg_hugr2().unwrap() } From 344d9835b28e8b8eebafa9a0c678adbddd5aea03 Mon Sep 17 00:00:00 2001 From: Seyon Sivarajah Date: Thu, 31 Aug 2023 11:03:36 +0100 Subject: [PATCH 8/9] add identity insertion tests --- src/hugr/rewrite/insert_identity.rs | 55 +++++++++++++++++++++++++---- 1 file changed, 48 insertions(+), 7 deletions(-) diff --git a/src/hugr/rewrite/insert_identity.rs b/src/hugr/rewrite/insert_identity.rs index cb4c26ac1..5816fdcf1 100644 --- a/src/hugr/rewrite/insert_identity.rs +++ b/src/hugr/rewrite/insert_identity.rs @@ -82,14 +82,55 @@ impl Rewrite for IdentityInsertion { #[cfg(test)] mod tests { - use rstest::{fixture, rstest}; + use rstest::rstest; - use crate::Hugr; + use super::super::simple_replace::test::dfg_hugr; + use super::*; + use crate::{ + algorithm::nest_cfgs::test::build_conditional_in_loop_cfg, extension::prelude::QB_T, + ops::handle::NodeHandle, Hugr, + }; - #[fixture] - fn sample() -> Hugr { - todo!() - } #[rstest] - fn correct_insertion() {} + fn correct_insertion(dfg_hugr: Hugr) { + let mut h = dfg_hugr; + + assert_eq!(h.node_count(), 6); + + let final_node = h + .input_neighbours(h.get_io(h.root()).unwrap()[1]) + .next() + .unwrap(); + + let final_node_port = h.node_inputs(final_node).next().unwrap(); + + let rw = IdentityInsertion::new(final_node, final_node_port); + + let noop_node = h.apply_rewrite(rw).unwrap(); + + assert_eq!(h.node_count(), 7); + + let noop: LeafOp = h.get_optype(noop_node).clone().try_into().unwrap(); + + assert_eq!(noop, LeafOp::Noop { ty: QB_T }); + } + + #[test] + fn incorrect_insertion() { + let (mut h, _, tail) = build_conditional_in_loop_cfg(false).unwrap(); + + let final_node = tail.node(); + + let final_node_port = h.node_inputs(final_node).next().unwrap(); + + let rw = IdentityInsertion::new(final_node, final_node_port); + + let apply_result = h.apply_rewrite(rw); + assert_eq!( + apply_result, + Err(IdentityInsertionError::InvalidPortKind(Some( + EdgeKind::ControlFlow + ))) + ); + } } From 21ac480297104f06f9957df6f8706ea3c8263d17 Mon Sep 17 00:00:00 2001 From: Seyon Sivarajah Date: Thu, 31 Aug 2023 11:19:09 +0100 Subject: [PATCH 9/9] add check port is output --- src/hugr/rewrite/insert_identity.rs | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/hugr/rewrite/insert_identity.rs b/src/hugr/rewrite/insert_identity.rs index 5816fdcf1..c2d177b29 100644 --- a/src/hugr/rewrite/insert_identity.rs +++ b/src/hugr/rewrite/insert_identity.rs @@ -3,7 +3,7 @@ use crate::hugr::{HugrMut, Node}; use crate::ops::LeafOp; use crate::types::EdgeKind; -use crate::{Hugr, HugrView, Port}; +use crate::{Direction, Hugr, HugrView, Port}; use super::Rewrite; @@ -38,6 +38,10 @@ pub enum IdentityInsertionError { /// Invalid port kind. #[error("post_port has invalid kind {0:?}. Must be Value.")] InvalidPortKind(Option), + + /// Must be input port. + #[error("post_port is an output port, must be input.")] + PortIsOutput, } impl Rewrite for IdentityInsertion { @@ -53,11 +57,15 @@ impl Rewrite for IdentityInsertion { Conditions: 1. post_port is Value kind. 2. post_port is connected to a sibling of post_node. + 3. post_port is input. */ unimplemented!() } fn apply(self, h: &mut Hugr) -> Result { + if self.post_port.direction() != Direction::Incoming { + return Err(IdentityInsertionError::PortIsOutput); + } let (pre_node, pre_port) = h .linked_ports(self.post_node, self.post_port) .exactly_one() @@ -73,7 +81,6 @@ impl Rewrite for IdentityInsertion { h.connect(pre_node, pre_port.index(), new_node, 0) .expect("Should only fail if ports don't exist."); - // TODO Check type, insert Noop... h.connect(new_node, 0, self.post_node, self.post_port.index()) .expect("Should only fail if ports don't exist."); Ok(new_node) @@ -121,9 +128,14 @@ mod tests { let final_node = tail.node(); - let final_node_port = h.node_inputs(final_node).next().unwrap(); + let final_node_output = h.node_outputs(final_node).next().unwrap(); + let rw = IdentityInsertion::new(final_node, final_node_output); + let apply_result = h.apply_rewrite(rw); + assert_eq!(apply_result, Err(IdentityInsertionError::PortIsOutput)); - let rw = IdentityInsertion::new(final_node, final_node_port); + let final_node_input = h.node_inputs(final_node).next().unwrap(); + + let rw = IdentityInsertion::new(final_node, final_node_input); let apply_result = h.apply_rewrite(rw); assert_eq!(