From 80703e193d32975b92b72e7f493379557655f829 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 5 Jun 2023 15:22:52 +0100 Subject: [PATCH 01/23] Remove Pattern --- src/rewrite/pattern.rs | 10 ---------- 1 file changed, 10 deletions(-) delete mode 100644 src/rewrite/pattern.rs diff --git a/src/rewrite/pattern.rs b/src/rewrite/pattern.rs deleted file mode 100644 index 85a3b8a4b..000000000 --- a/src/rewrite/pattern.rs +++ /dev/null @@ -1,10 +0,0 @@ -#![allow(missing_docs)] -//! Pattern matching operations on a HUGR. - -#[cfg(feature = "pyo3")] -use pyo3::prelude::*; - -#[derive(Clone, Debug, PartialEq, Eq)] -#[cfg_attr(feature = "pyo3", pyclass)] -#[non_exhaustive] -pub struct Pattern {} From 9ef30ad4cf00951dfd1ed80893cde9ebb5e8ad22 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 5 Jun 2023 15:24:25 +0100 Subject: [PATCH 02/23] Remove Pattern.rs, too skeletal to be useful --- src/rewrite.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/rewrite.rs b/src/rewrite.rs index dfc2182bf..5e48dffa4 100644 --- a/src/rewrite.rs +++ b/src/rewrite.rs @@ -1,6 +1,5 @@ -//! Pattern matching and rewrite operations on the HUGR. +//! Rewrite operations on the HUGR. -pub mod pattern; #[allow(clippy::module_inception)] // TODO: Rename? pub mod rewrite; From 75b9f1f24da90a92664b50b8133b1fefabd16453 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 5 Jun 2023 15:27:26 +0100 Subject: [PATCH 03/23] Move rewrite to hugr/replace --- src/hugr.rs | 3 ++- src/{rewrite.rs => hugr/replace.rs} | 0 src/{rewrite => hugr/replace}/rewrite.rs | 0 src/lib.rs | 4 +--- 4 files changed, 3 insertions(+), 4 deletions(-) rename src/{rewrite.rs => hugr/replace.rs} (100%) rename src/{rewrite => hugr/replace}/rewrite.rs (100%) diff --git a/src/hugr.rs b/src/hugr.rs index 9d3094fb9..7f1b68e68 100644 --- a/src/hugr.rs +++ b/src/hugr.rs @@ -5,10 +5,12 @@ pub mod view; pub mod serialize; pub mod validate; +pub mod replace; use derive_more::From; pub use hugrmut::HugrMut; pub use validate::ValidationError; +pub use replace::{Rewrite, RewriteError}; use portgraph::dot::{hier_graph_dot_string_with, DotEdgeStyle}; use portgraph::{Hierarchy, PortGraph, UnmanagedDenseMap}; @@ -16,7 +18,6 @@ use thiserror::Error; pub use self::view::HugrView; use crate::ops::{ModuleOp, OpType}; -use crate::rewrite::{Rewrite, RewriteError}; use crate::types::EdgeKind; use html_escape::encode_text_to_string; diff --git a/src/rewrite.rs b/src/hugr/replace.rs similarity index 100% rename from src/rewrite.rs rename to src/hugr/replace.rs diff --git a/src/rewrite/rewrite.rs b/src/hugr/replace/rewrite.rs similarity index 100% rename from src/rewrite/rewrite.rs rename to src/hugr/replace/rewrite.rs diff --git a/src/lib.rs b/src/lib.rs index e59acc115..d76703918 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -16,10 +16,8 @@ pub mod hugr; pub mod macros; pub mod ops; pub mod resource; -pub mod rewrite; pub mod types; mod utils; -pub use crate::hugr::{Direction, Hugr, Node, Port, Wire}; +pub use crate::hugr::{Direction, Hugr, Node, Port, Wire, Rewrite, RewriteError}; pub use crate::resource::Resource; -pub use crate::rewrite::{Rewrite, RewriteError}; From 5c5662fd5da6d06b37a48835fa4a9ae036fc3cab Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 5 Jun 2023 15:45:27 +0100 Subject: [PATCH 04/23] Add RewriteOp enum, move Hugr code into RewriteOp::apply (a big match) --- src/hugr.rs | 29 ++++------------------------- src/hugr/replace.rs | 44 +++++++++++++++++++++++++++++++++++++++++++- src/lib.rs | 2 +- 3 files changed, 48 insertions(+), 27 deletions(-) diff --git a/src/hugr.rs b/src/hugr.rs index 7f1b68e68..8033889c9 100644 --- a/src/hugr.rs +++ b/src/hugr.rs @@ -3,14 +3,14 @@ mod hugrmut; pub mod view; +pub mod replace; pub mod serialize; pub mod validate; -pub mod replace; use derive_more::From; pub use hugrmut::HugrMut; +pub use replace::{Rewrite, RewriteError, RewriteOp}; pub use validate::ValidationError; -pub use replace::{Rewrite, RewriteError}; use portgraph::dot::{hier_graph_dot_string_with, DotEdgeStyle}; use portgraph::{Hierarchy, PortGraph, UnmanagedDenseMap}; @@ -75,29 +75,8 @@ impl Hugr { } /// Applies a rewrite to the graph. - pub fn apply_rewrite(mut self, rewrite: Rewrite) -> Result<(), RewriteError> { - // Get the open graph for the rewrites, and a HUGR with the additional components. - let (rewrite, mut replacement, parents) = rewrite.into_parts(); - - // TODO: Use `parents` to update the hierarchy, and keep the internal hierarchy from `replacement`. - let _ = parents; - - let node_inserted = |old, new| { - std::mem::swap(&mut self.op_types[new], &mut replacement.op_types[old]); - // TODO: metadata (Fn parameter ?) - }; - rewrite.apply_with_callbacks( - &mut self.graph, - |_| {}, - |_| {}, - node_inserted, - |_, _| {}, - |_, _| {}, - )?; - - // TODO: Check types - - Ok(()) + pub fn apply(&mut self, op: RewriteOp) -> Result<(), RewriteError> { + op.apply(self) } /// Return dot string showing underlying graph and hierarchy side by side. diff --git a/src/hugr/replace.rs b/src/hugr/replace.rs index 5e48dffa4..178745de4 100644 --- a/src/hugr/replace.rs +++ b/src/hugr/replace.rs @@ -2,5 +2,47 @@ #[allow(clippy::module_inception)] // TODO: Rename? pub mod rewrite; - +use crate::Hugr; pub use rewrite::{OpenHugr, Rewrite, RewriteError}; + +/// An operation that can be applied to mutate a Hugr +pub enum RewriteOp { + /// Replace a set of dataflow sibling nodes with new ones + Rewrite(Rewrite), + /// Move a set of controlflow sibling nodes into a nested CFG + OutlineCFG, +} + +impl RewriteOp { + /// Applies a ReplacementOp to the graph. + pub fn apply(self, h: &mut Hugr) -> Result<(), RewriteError> { + match self { + Self::Rewrite(rewrite) => { + // Get the open graph for the rewrites, and a HUGR with the additional components. + let (rewrite, mut replacement, parents) = rewrite.into_parts(); + + // TODO: Use `parents` to update the hierarchy, and keep the internal hierarchy from `replacement`. + let _ = parents; + + let node_inserted = |old, new| { + std::mem::swap(&mut h.op_types[new], &mut replacement.op_types[old]); + // TODO: metadata (Fn parameter ?) + }; + rewrite.apply_with_callbacks( + &mut h.graph, + |_| {}, + |_| {}, + node_inserted, + |_, _| {}, + |_, _| {}, + )?; + + // TODO: Check types + } + Self::OutlineCFG => { + todo!(); + } + }; + Ok(()) + } +} diff --git a/src/lib.rs b/src/lib.rs index d76703918..6fdc95fd6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -19,5 +19,5 @@ pub mod resource; pub mod types; mod utils; -pub use crate::hugr::{Direction, Hugr, Node, Port, Wire, Rewrite, RewriteError}; +pub use crate::hugr::{Direction, Hugr, Node, Port, Rewrite, RewriteError, Wire}; pub use crate::resource::Resource; From de703839ff338c6a59189ef14750280d1744cbee Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 5 Jun 2023 15:53:15 +0100 Subject: [PATCH 05/23] Make into a trait. So it has to be public...so what? --- src/hugr.rs | 2 +- src/hugr/replace.rs | 44 +++++-------------------------------- src/hugr/replace/rewrite.rs | 29 ++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 40 deletions(-) diff --git a/src/hugr.rs b/src/hugr.rs index 8033889c9..7999312ee 100644 --- a/src/hugr.rs +++ b/src/hugr.rs @@ -75,7 +75,7 @@ impl Hugr { } /// Applies a rewrite to the graph. - pub fn apply(&mut self, op: RewriteOp) -> Result<(), RewriteError> { + pub fn apply(&mut self, op: impl RewriteOp) -> Result<(), RewriteError> { op.apply(self) } diff --git a/src/hugr/replace.rs b/src/hugr/replace.rs index 178745de4..4d016d1fd 100644 --- a/src/hugr/replace.rs +++ b/src/hugr/replace.rs @@ -6,43 +6,9 @@ use crate::Hugr; pub use rewrite::{OpenHugr, Rewrite, RewriteError}; /// An operation that can be applied to mutate a Hugr -pub enum RewriteOp { - /// Replace a set of dataflow sibling nodes with new ones - Rewrite(Rewrite), - /// Move a set of controlflow sibling nodes into a nested CFG - OutlineCFG, -} - -impl RewriteOp { - /// Applies a ReplacementOp to the graph. - pub fn apply(self, h: &mut Hugr) -> Result<(), RewriteError> { - match self { - Self::Rewrite(rewrite) => { - // Get the open graph for the rewrites, and a HUGR with the additional components. - let (rewrite, mut replacement, parents) = rewrite.into_parts(); - - // TODO: Use `parents` to update the hierarchy, and keep the internal hierarchy from `replacement`. - let _ = parents; - - let node_inserted = |old, new| { - std::mem::swap(&mut h.op_types[new], &mut replacement.op_types[old]); - // TODO: metadata (Fn parameter ?) - }; - rewrite.apply_with_callbacks( - &mut h.graph, - |_| {}, - |_| {}, - node_inserted, - |_, _| {}, - |_, _| {}, - )?; - - // TODO: Check types - } - Self::OutlineCFG => { - todo!(); - } - }; - Ok(()) - } +pub trait RewriteOp { + /// Mutate the specified Hugr, or fail with a RewriteError. + /// Implementations are strongly encouraged not to mutate the Hugr + /// if they return an Err. + fn apply(self, h: &mut Hugr) -> Result<(), RewriteError>; } diff --git a/src/hugr/replace/rewrite.rs b/src/hugr/replace/rewrite.rs index e08e95b50..f11aec476 100644 --- a/src/hugr/replace/rewrite.rs +++ b/src/hugr/replace/rewrite.rs @@ -8,6 +8,7 @@ use portgraph::{NodeIndex, PortIndex}; use thiserror::Error; use crate::Hugr; +use super::RewriteOp; /// A subset of the nodes in a graph, and the ports that it is connected to. #[derive(Debug, Clone, Default)] @@ -131,6 +132,34 @@ impl Rewrite { } } +impl RewriteOp for Rewrite { + /// Applies a ReplacementOp to the graph. + fn apply(self, h: &mut Hugr) -> Result<(), RewriteError> { + // Get the open graph for the rewrites, and a HUGR with the additional components. + let (rewrite, mut replacement, parents) = self.into_parts(); + + // TODO: Use `parents` to update the hierarchy, and keep the internal hierarchy from `replacement`. + let _ = parents; + + let node_inserted = |old, new| { + std::mem::swap(&mut h.op_types[new], &mut replacement.op_types[old]); + // TODO: metadata (Fn parameter ?) + }; + rewrite.apply_with_callbacks( + &mut h.graph, + |_| {}, + |_| {}, + node_inserted, + |_, _| {}, + |_, _| {}, + )?; + + // TODO: Check types + Ok(()) + } +} + + /// Error generated when a rewrite fails. #[derive(Debug, Clone, Error, PartialEq, Eq)] pub enum RewriteError { From 3a534e69cb7b1bad412ee6407d411f400b13d529 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 5 Jun 2023 15:55:02 +0100 Subject: [PATCH 06/23] Parametrize by error type --- src/hugr.rs | 2 +- src/hugr/replace.rs | 6 +++--- src/hugr/replace/rewrite.rs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/hugr.rs b/src/hugr.rs index 7999312ee..a12fc8cf5 100644 --- a/src/hugr.rs +++ b/src/hugr.rs @@ -75,7 +75,7 @@ impl Hugr { } /// Applies a rewrite to the graph. - pub fn apply(&mut self, op: impl RewriteOp) -> Result<(), RewriteError> { + pub fn apply(&mut self, op: impl RewriteOp) -> Result<(), E> { op.apply(self) } diff --git a/src/hugr/replace.rs b/src/hugr/replace.rs index 4d016d1fd..964c91947 100644 --- a/src/hugr/replace.rs +++ b/src/hugr/replace.rs @@ -6,9 +6,9 @@ use crate::Hugr; pub use rewrite::{OpenHugr, Rewrite, RewriteError}; /// An operation that can be applied to mutate a Hugr -pub trait RewriteOp { - /// Mutate the specified Hugr, or fail with a RewriteError. +pub trait RewriteOp { + /// Mutate the specified Hugr, or fail with an error. /// Implementations are strongly encouraged not to mutate the Hugr /// if they return an Err. - fn apply(self, h: &mut Hugr) -> Result<(), RewriteError>; + fn apply(self, h: &mut Hugr) -> Result<(), E>; } diff --git a/src/hugr/replace/rewrite.rs b/src/hugr/replace/rewrite.rs index f11aec476..f3c119e52 100644 --- a/src/hugr/replace/rewrite.rs +++ b/src/hugr/replace/rewrite.rs @@ -132,7 +132,7 @@ impl Rewrite { } } -impl RewriteOp for Rewrite { +impl RewriteOp for Rewrite { /// Applies a ReplacementOp to the graph. fn apply(self, h: &mut Hugr) -> Result<(), RewriteError> { // Get the open graph for the rewrites, and a HUGR with the additional components. From 55f83baa9a68d31d379de1e6f668e671afece49d Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 5 Jun 2023 16:30:06 +0100 Subject: [PATCH 07/23] fmt --- src/hugr/replace/rewrite.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/hugr/replace/rewrite.rs b/src/hugr/replace/rewrite.rs index f3c119e52..9d3222dba 100644 --- a/src/hugr/replace/rewrite.rs +++ b/src/hugr/replace/rewrite.rs @@ -7,8 +7,8 @@ use portgraph::substitute::OpenGraph; use portgraph::{NodeIndex, PortIndex}; use thiserror::Error; -use crate::Hugr; use super::RewriteOp; +use crate::Hugr; /// A subset of the nodes in a graph, and the ports that it is connected to. #[derive(Debug, Clone, Default)] @@ -159,7 +159,6 @@ impl RewriteOp for Rewrite { } } - /// Error generated when a rewrite fails. #[derive(Debug, Clone, Error, PartialEq, Eq)] pub enum RewriteError { From 76d13fe9552b6ba4451d1207b73499d9fde7fdbc Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 6 Jun 2023 09:03:45 +0100 Subject: [PATCH 08/23] Rename hugr/replace/{rewrite.rs -> replace.rs} --- src/hugr/replace.rs | 4 ++-- src/hugr/replace/{rewrite.rs => replace.rs} | 0 2 files changed, 2 insertions(+), 2 deletions(-) rename src/hugr/replace/{rewrite.rs => replace.rs} (100%) diff --git a/src/hugr/replace.rs b/src/hugr/replace.rs index 964c91947..94c7b3c85 100644 --- a/src/hugr/replace.rs +++ b/src/hugr/replace.rs @@ -1,9 +1,9 @@ //! Rewrite operations on the HUGR. #[allow(clippy::module_inception)] // TODO: Rename? -pub mod rewrite; +pub mod replace; use crate::Hugr; -pub use rewrite::{OpenHugr, Rewrite, RewriteError}; +pub use replace::{OpenHugr, Rewrite, RewriteError}; /// An operation that can be applied to mutate a Hugr pub trait RewriteOp { diff --git a/src/hugr/replace/rewrite.rs b/src/hugr/replace/replace.rs similarity index 100% rename from src/hugr/replace/rewrite.rs rename to src/hugr/replace/replace.rs From 6e8b1525b0f0f4cb3bec74953466d2eba8eda64a Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 6 Jun 2023 09:05:29 +0100 Subject: [PATCH 09/23] Rename src/hugr/{replace=>rewrite}(,.rs) --- src/hugr.rs | 4 ++-- src/hugr/{replace.rs => rewrite.rs} | 0 src/hugr/{replace => rewrite}/replace.rs | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename src/hugr/{replace.rs => rewrite.rs} (100%) rename src/hugr/{replace => rewrite}/replace.rs (100%) diff --git a/src/hugr.rs b/src/hugr.rs index a12fc8cf5..7cb34959d 100644 --- a/src/hugr.rs +++ b/src/hugr.rs @@ -3,13 +3,13 @@ mod hugrmut; pub mod view; -pub mod replace; +pub mod rewrite; pub mod serialize; pub mod validate; use derive_more::From; pub use hugrmut::HugrMut; -pub use replace::{Rewrite, RewriteError, RewriteOp}; +pub use rewrite::{Rewrite, RewriteError, RewriteOp}; pub use validate::ValidationError; use portgraph::dot::{hier_graph_dot_string_with, DotEdgeStyle}; diff --git a/src/hugr/replace.rs b/src/hugr/rewrite.rs similarity index 100% rename from src/hugr/replace.rs rename to src/hugr/rewrite.rs diff --git a/src/hugr/replace/replace.rs b/src/hugr/rewrite/replace.rs similarity index 100% rename from src/hugr/replace/replace.rs rename to src/hugr/rewrite/replace.rs From 99b31ce96f6bd9b42232f3c2357e8c1fa20ef9f0 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 6 Jun 2023 09:10:51 +0100 Subject: [PATCH 10/23] Rename Rewrite(Error) to Replace(Error) --- src/hugr.rs | 2 +- src/hugr/rewrite.rs | 4 ++-- src/hugr/rewrite/replace.rs | 26 +++++++++++++------------- src/lib.rs | 2 +- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/hugr.rs b/src/hugr.rs index 7cb34959d..09a2a0516 100644 --- a/src/hugr.rs +++ b/src/hugr.rs @@ -9,7 +9,7 @@ pub mod validate; use derive_more::From; pub use hugrmut::HugrMut; -pub use rewrite::{Rewrite, RewriteError, RewriteOp}; +pub use rewrite::{Replace, ReplaceError, RewriteOp}; pub use validate::ValidationError; use portgraph::dot::{hier_graph_dot_string_with, DotEdgeStyle}; diff --git a/src/hugr/rewrite.rs b/src/hugr/rewrite.rs index 94c7b3c85..b8722de2c 100644 --- a/src/hugr/rewrite.rs +++ b/src/hugr/rewrite.rs @@ -1,9 +1,9 @@ -//! Rewrite operations on the HUGR. +//! Replace operations on the HUGR. #[allow(clippy::module_inception)] // TODO: Rename? pub mod replace; use crate::Hugr; -pub use replace::{OpenHugr, Rewrite, RewriteError}; +pub use replace::{OpenHugr, Replace, ReplaceError}; /// An operation that can be applied to mutate a Hugr pub trait RewriteOp { diff --git a/src/hugr/rewrite/replace.rs b/src/hugr/rewrite/replace.rs index 9d3222dba..05c7574bb 100644 --- a/src/hugr/rewrite/replace.rs +++ b/src/hugr/rewrite/replace.rs @@ -1,5 +1,5 @@ #![allow(missing_docs)] -//! Rewrite operations on Hugr graphs. +//! Replace operations on Hugr graphs. use std::collections::HashMap; @@ -77,7 +77,7 @@ pub type ParentsMap = HashMap; /// A rewrite operation that replaces a subgraph with another graph. /// Includes the new weights for the nodes in the replacement graph. #[derive(Debug, Clone)] -pub struct Rewrite { +pub struct Replace { /// The subgraph to be replaced. subgraph: BoundedSubgraph, /// The replacement graph. @@ -86,7 +86,7 @@ pub struct Rewrite { parents: ParentsMap, } -impl Rewrite { +impl Replace { /// Creates a new rewrite operation. pub fn new( subgraph: BoundedSubgraph, @@ -117,24 +117,24 @@ impl Rewrite { /// /// This includes having a convex subgraph (TODO: include definition), and /// having matching numbers of ports on the boundaries. - pub fn verify(&self) -> Result<(), RewriteError> { + pub fn verify(&self) -> Result<(), ReplaceError> { self.verify_convexity()?; self.verify_boundaries()?; Ok(()) } - pub fn verify_convexity(&self) -> Result<(), RewriteError> { + pub fn verify_convexity(&self) -> Result<(), ReplaceError> { todo!() } - pub fn verify_boundaries(&self) -> Result<(), RewriteError> { + pub fn verify_boundaries(&self) -> Result<(), ReplaceError> { todo!() } } -impl RewriteOp for Rewrite { - /// Applies a ReplacementOp to the graph. - fn apply(self, h: &mut Hugr) -> Result<(), RewriteError> { +impl RewriteOp for Replace { + /// Performs a Replace operation on the graph. + fn apply(self, h: &mut Hugr) -> Result<(), ReplaceError> { // Get the open graph for the rewrites, and a HUGR with the additional components. let (rewrite, mut replacement, parents) = self.into_parts(); @@ -161,9 +161,9 @@ impl RewriteOp for Rewrite { /// Error generated when a rewrite fails. #[derive(Debug, Clone, Error, PartialEq, Eq)] -pub enum RewriteError { - /// The rewrite failed because the boundary defined by the - /// [`Rewrite`] could not be matched to the dangling ports of the +pub enum ReplaceError { + /// The replacement failed because the boundary defined by the + /// [`Replace`] could not be matched to the dangling ports of the /// [`OpenHugr`]. #[error("The boundary defined by the rewrite could not be matched to the dangling ports of the OpenHugr")] BoundarySize(#[source] portgraph::substitute::RewriteError), @@ -178,7 +178,7 @@ pub enum RewriteError { NotConvex(), } -impl From for RewriteError { +impl From for ReplaceError { fn from(e: portgraph::substitute::RewriteError) -> Self { match e { portgraph::substitute::RewriteError::BoundarySize => Self::BoundarySize(e), diff --git a/src/lib.rs b/src/lib.rs index 6fdc95fd6..d4a01c7b0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -19,5 +19,5 @@ pub mod resource; pub mod types; mod utils; -pub use crate::hugr::{Direction, Hugr, Node, Port, Rewrite, RewriteError, Wire}; +pub use crate::hugr::{Direction, Hugr, Node, Port, Replace, ReplaceError, Wire}; pub use crate::resource::Resource; From 070afdd69e0f8bc01bb6bd6eb51d5df5866d5fc2 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 6 Jun 2023 09:15:33 +0100 Subject: [PATCH 11/23] Rename RewriteOp to Rewrite --- src/hugr.rs | 4 ++-- src/hugr/rewrite.rs | 2 +- src/hugr/rewrite/replace.rs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/hugr.rs b/src/hugr.rs index 09a2a0516..0c89aceb0 100644 --- a/src/hugr.rs +++ b/src/hugr.rs @@ -9,7 +9,7 @@ pub mod validate; use derive_more::From; pub use hugrmut::HugrMut; -pub use rewrite::{Replace, ReplaceError, RewriteOp}; +pub use rewrite::{Replace, ReplaceError, Rewrite}; pub use validate::ValidationError; use portgraph::dot::{hier_graph_dot_string_with, DotEdgeStyle}; @@ -75,7 +75,7 @@ impl Hugr { } /// Applies a rewrite to the graph. - pub fn apply(&mut self, op: impl RewriteOp) -> Result<(), E> { + pub fn apply(&mut self, op: impl Rewrite) -> Result<(), E> { op.apply(self) } diff --git a/src/hugr/rewrite.rs b/src/hugr/rewrite.rs index b8722de2c..dfffd8ece 100644 --- a/src/hugr/rewrite.rs +++ b/src/hugr/rewrite.rs @@ -6,7 +6,7 @@ use crate::Hugr; pub use replace::{OpenHugr, Replace, ReplaceError}; /// An operation that can be applied to mutate a Hugr -pub trait RewriteOp { +pub trait Rewrite { /// Mutate the specified Hugr, or fail with an error. /// Implementations are strongly encouraged not to mutate the Hugr /// if they return an Err. diff --git a/src/hugr/rewrite/replace.rs b/src/hugr/rewrite/replace.rs index 05c7574bb..4c8cc1d9c 100644 --- a/src/hugr/rewrite/replace.rs +++ b/src/hugr/rewrite/replace.rs @@ -7,7 +7,7 @@ use portgraph::substitute::OpenGraph; use portgraph::{NodeIndex, PortIndex}; use thiserror::Error; -use super::RewriteOp; +use super::Rewrite; use crate::Hugr; /// A subset of the nodes in a graph, and the ports that it is connected to. @@ -132,7 +132,7 @@ impl Replace { } } -impl RewriteOp for Replace { +impl Rewrite for Replace { /// Performs a Replace operation on the graph. fn apply(self, h: &mut Hugr) -> Result<(), ReplaceError> { // Get the open graph for the rewrites, and a HUGR with the additional components. From 7348e1178fd43ed2d1880cb693f8fe4a2c9d015c Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 6 Jun 2023 09:16:23 +0100 Subject: [PATCH 12/23] Hugr::apply -> apply_rewrite --- src/hugr.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hugr.rs b/src/hugr.rs index 0c89aceb0..416f9790f 100644 --- a/src/hugr.rs +++ b/src/hugr.rs @@ -75,8 +75,8 @@ impl Hugr { } /// Applies a rewrite to the graph. - pub fn apply(&mut self, op: impl Rewrite) -> Result<(), E> { - op.apply(self) + pub fn apply_rewrite(&mut self, rw: impl Rewrite) -> Result<(), E> { + rw.apply(self) } /// Return dot string showing underlying graph and hierarchy side by side. From ec8354ea9f4a3ad3a9d5da39514eb305f037b2db Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 6 Jun 2023 15:13:01 +0100 Subject: [PATCH 13/23] Add may_fail_destructively check, default true, and Transactional wrapper --- src/hugr/rewrite.rs | 37 +++++++++++++++++++++++++++++++++++-- src/hugr/rewrite/replace.rs | 5 ++++- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/src/hugr/rewrite.rs b/src/hugr/rewrite.rs index dfffd8ece..094f244d1 100644 --- a/src/hugr/rewrite.rs +++ b/src/hugr/rewrite.rs @@ -2,13 +2,46 @@ #[allow(clippy::module_inception)] // TODO: Rename? pub mod replace; +use std::mem; + use crate::Hugr; pub use replace::{OpenHugr, Replace, ReplaceError}; /// An operation that can be applied to mutate a Hugr pub trait Rewrite { + /// Check that this operation can be applied to the specified Hugr in a way + /// that will not destroy it. See contract for [self.apply]. + fn may_fail_destructively(&self, _h: &Hugr) -> bool { + true // The least guarantee possible. + } + /// Mutate the specified Hugr, or fail with an error. - /// Implementations are strongly encouraged not to mutate the Hugr - /// if they return an Err. + /// [self.may_fail_destructively] may be used before calling in order to rule out destructive failure. + /// If [self.may_fail_destructively] returns true, or `h` has since been mutated, + /// or `h` does not [Hugr::validate], then no guarantees are given here. + /// However if [self.may_fail_destructively] returns false, `h` has not been mutated since then, + /// and `h` [Hugr::validate]s, then this method must *either* succeed, *or* fail leaving `h` unchanged. fn apply(self, h: &mut Hugr) -> Result<(), E>; } + +/// Wraps any rewrite into a transaction (i.e. that has no effect upon failure) +pub struct Transactional { + underlying: R, +} + +impl> Rewrite for Transactional { + fn may_fail_destructively(&self, _h: &Hugr) -> bool { + false + } + + fn apply(self, h: &mut Hugr) -> Result<(), E> { + // note that if underlying.may_fail_destructively(h) is false, we don't need a backup... + let backup = h.clone(); + let r = self.underlying.apply(h); + if let Err(_) = r { + // drop the old h, it was undefined + let _ = mem::replace(h, backup); + } + r + } +} diff --git a/src/hugr/rewrite/replace.rs b/src/hugr/rewrite/replace.rs index 4c8cc1d9c..f78fcbeef 100644 --- a/src/hugr/rewrite/replace.rs +++ b/src/hugr/rewrite/replace.rs @@ -1,5 +1,6 @@ #![allow(missing_docs)] -//! Replace operations on Hugr graphs. +//! Replace operations on Hugr graphs. This is a nonfunctional +//! dummy implementation just to demonstrate design principles. use std::collections::HashMap; @@ -117,6 +118,7 @@ impl Replace { /// /// This includes having a convex subgraph (TODO: include definition), and /// having matching numbers of ports on the boundaries. + /// TODO does this relate to [Rewrite::may_fail_destructively]? It probably should! pub fn verify(&self) -> Result<(), ReplaceError> { self.verify_convexity()?; self.verify_boundaries()?; @@ -145,6 +147,7 @@ impl Rewrite for Replace { std::mem::swap(&mut h.op_types[new], &mut replacement.op_types[old]); // TODO: metadata (Fn parameter ?) }; + // Our (default) may_fail_destructively() returns true, so no guarantees here rewrite.apply_with_callbacks( &mut h.graph, |_| {}, From 147c9374f644894cb61c59e35393043fea435863 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 6 Jun 2023 15:16:20 +0100 Subject: [PATCH 14/23] is_err --- src/hugr/rewrite.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hugr/rewrite.rs b/src/hugr/rewrite.rs index 094f244d1..14a3760da 100644 --- a/src/hugr/rewrite.rs +++ b/src/hugr/rewrite.rs @@ -38,7 +38,7 @@ impl> Rewrite for Transactional { // note that if underlying.may_fail_destructively(h) is false, we don't need a backup... let backup = h.clone(); let r = self.underlying.apply(h); - if let Err(_) = r { + if r.is_err() { // drop the old h, it was undefined let _ = mem::replace(h, backup); } From 9f1d3826371a53a8344df8b47327db6bd088f2e5 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Wed, 7 Jun 2023 12:09:14 +0100 Subject: [PATCH 15/23] unchanged_on_failure as trait associated constant --- src/hugr/rewrite.rs | 34 ++++++++++++++++++++++------------ src/hugr/rewrite/replace.rs | 27 +++++++++++++++------------ 2 files changed, 37 insertions(+), 24 deletions(-) diff --git a/src/hugr/rewrite.rs b/src/hugr/rewrite.rs index 14a3760da..f4c9ce487 100644 --- a/src/hugr/rewrite.rs +++ b/src/hugr/rewrite.rs @@ -9,18 +9,21 @@ pub use replace::{OpenHugr, Replace, ReplaceError}; /// An operation that can be applied to mutate a Hugr pub trait Rewrite { - /// Check that this operation can be applied to the specified Hugr in a way - /// that will not destroy it. See contract for [self.apply]. - fn may_fail_destructively(&self, _h: &Hugr) -> bool { - true // The least guarantee possible. - } + /// If `true`, [self.apply]'s of this rewrite guarantee that they do not mutate the Hugr when they return an Err. + /// If `false`, there is no guarantee; the Hugr should be assumed invalid when Err is returned. + const UNCHANGED_ON_FAILURE: bool; + + /// Checks whether the rewrite would succeed on the specified Hugr. + /// If this call succeeds, [self.apply] should also succeed on the same `h` + /// If this calls fails, [self.apply] would fail with the same error. + fn verify(&self, h: &Hugr) -> Result<(), E>; /// Mutate the specified Hugr, or fail with an error. - /// [self.may_fail_destructively] may be used before calling in order to rule out destructive failure. - /// If [self.may_fail_destructively] returns true, or `h` has since been mutated, - /// or `h` does not [Hugr::validate], then no guarantees are given here. - /// However if [self.may_fail_destructively] returns false, `h` has not been mutated since then, - /// and `h` [Hugr::validate]s, then this method must *either* succeed, *or* fail leaving `h` unchanged. + /// If [self.unchanged_on_failure] is true, then `h` must be unchanged if Err is returned. + /// See also [self.verify] + /// # Panics + /// May panic if-and-only-if `h` would have failed [Hugr::validate]; that is, + /// implementations may begin with `assert!(h.validate())`, and *should* begin with `debug_assert!(h.validate())` fn apply(self, h: &mut Hugr) -> Result<(), E>; } @@ -29,12 +32,19 @@ pub struct Transactional { underlying: R, } +// Note we might like to constrain R to Rewrite but this +// is not yet supported, https://github.com/rust-lang/rust/issues/92827 impl> Rewrite for Transactional { - fn may_fail_destructively(&self, _h: &Hugr) -> bool { - false + const UNCHANGED_ON_FAILURE: bool = true; + + fn verify(&self, h: &Hugr) -> Result<(), E> { + self.underlying.verify(h) } fn apply(self, h: &mut Hugr) -> Result<(), E> { + if R::UNCHANGED_ON_FAILURE { + return self.underlying.apply(h); + } // note that if underlying.may_fail_destructively(h) is false, we don't need a backup... let backup = h.clone(); let r = self.underlying.apply(h); diff --git a/src/hugr/rewrite/replace.rs b/src/hugr/rewrite/replace.rs index f78fcbeef..ae9ec1a24 100644 --- a/src/hugr/rewrite/replace.rs +++ b/src/hugr/rewrite/replace.rs @@ -114,17 +114,6 @@ impl Replace { ) } - /// Checks that the rewrite is valid. - /// - /// This includes having a convex subgraph (TODO: include definition), and - /// having matching numbers of ports on the boundaries. - /// TODO does this relate to [Rewrite::may_fail_destructively]? It probably should! - pub fn verify(&self) -> Result<(), ReplaceError> { - self.verify_convexity()?; - self.verify_boundaries()?; - Ok(()) - } - pub fn verify_convexity(&self) -> Result<(), ReplaceError> { todo!() } @@ -135,6 +124,20 @@ impl Replace { } impl Rewrite for Replace { + const UNCHANGED_ON_FAILURE: bool = false; + + /// Checks that the rewrite is valid. + /// + /// This includes having a convex subgraph (TODO: include definition), and + /// having matching numbers of ports on the boundaries. + /// TODO not clear this implementation really provides much guarantee about [self.apply] + /// but this class is not really working anyway. + fn verify(&self, _h: &Hugr) -> Result<(), ReplaceError> { + self.verify_convexity()?; + self.verify_boundaries()?; + Ok(()) + } + /// Performs a Replace operation on the graph. fn apply(self, h: &mut Hugr) -> Result<(), ReplaceError> { // Get the open graph for the rewrites, and a HUGR with the additional components. @@ -147,7 +150,7 @@ impl Rewrite for Replace { std::mem::swap(&mut h.op_types[new], &mut replacement.op_types[old]); // TODO: metadata (Fn parameter ?) }; - // Our (default) may_fail_destructively() returns true, so no guarantees here + // unchanged_on_failure is false, so no guarantees here rewrite.apply_with_callbacks( &mut h.graph, |_| {}, From f0b5b827c8538ba7f747ba99ee32ef953a0b4fbb Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Wed, 7 Jun 2023 12:19:00 +0100 Subject: [PATCH 16/23] Rephrase assert/debug_assert --- src/hugr/rewrite.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/hugr/rewrite.rs b/src/hugr/rewrite.rs index f4c9ce487..8455facff 100644 --- a/src/hugr/rewrite.rs +++ b/src/hugr/rewrite.rs @@ -23,7 +23,8 @@ pub trait Rewrite { /// See also [self.verify] /// # Panics /// May panic if-and-only-if `h` would have failed [Hugr::validate]; that is, - /// implementations may begin with `assert!(h.validate())`, and *should* begin with `debug_assert!(h.validate())` + /// implementations may begin with `assert!(h.validate())`, with `debug_assert!(h.validate())` + /// being preferred. fn apply(self, h: &mut Hugr) -> Result<(), E>; } From 4604c708d46a23b2830b849a2848a701e6b47fc9 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Fri, 9 Jun 2023 12:08:37 +0100 Subject: [PATCH 17/23] Move SimpleReplacement inside rewrite, move Hugr::apply_simple_replacement there --- src/hugr.rs | 192 +--------------- src/hugr/rewrite.rs | 4 +- .../rewrite}/simple_replace.rs | 205 +++++++++++++++++- src/lib.rs | 6 +- 4 files changed, 210 insertions(+), 197 deletions(-) rename src/{replacement => hugr/rewrite}/simple_replace.rs (59%) diff --git a/src/hugr.rs b/src/hugr.rs index 1ebfe4b62..60f0f3273 100644 --- a/src/hugr.rs +++ b/src/hugr.rs @@ -9,23 +9,19 @@ pub mod typecheck; pub mod validate; pub mod view; -use std::collections::HashMap; - pub use self::hugrmut::HugrMut; use self::multiportgraph::MultiPortGraph; pub use self::validate::ValidationError; use derive_more::From; -pub use rewrite::{Replace, ReplaceError, Rewrite}; +pub use rewrite::{Replace, ReplaceError, Rewrite, SimpleReplacement, SimpleReplacementError}; use portgraph::dot::{hier_graph_dot_string_with, DotEdgeStyle}; -use portgraph::{Hierarchy, NodeIndex, UnmanagedDenseMap}; +use portgraph::{Hierarchy, UnmanagedDenseMap}; use thiserror::Error; pub use self::view::HugrView; -use crate::ops::tag::OpTag; use crate::ops::{OpName, OpTrait, OpType}; -use crate::replacement::{SimpleReplacement, SimpleReplacementError}; use crate::types::EdgeKind; use html_escape::encode_text_to_string; @@ -74,190 +70,6 @@ pub struct Wire(Node, usize); /// Public API for HUGRs. impl Hugr { - /// Apply a simple replacement operation to the HUGR. - pub fn apply_simple_replacement( - &mut self, - r: SimpleReplacement, - ) -> Result<(), SimpleReplacementError> { - // 1. Check the parent node exists and is a DFG node. - if self.get_optype(r.parent).tag() != OpTag::Dfg { - return Err(SimpleReplacementError::InvalidParentNode()); - } - // 2. Check that all the to-be-removed nodes are children of it and are leaves. - for node in &r.removal { - if self.hierarchy.parent(node.index) != Some(r.parent.index) - || self.hierarchy.has_children(node.index) - { - return Err(SimpleReplacementError::InvalidRemovedNode()); - } - } - // 3. Do the replacement. - // 3.1. Add copies of all replacement nodes and edges to self. Exclude Input/Output nodes. - // Create map from old NodeIndex (in r.replacement) to new NodeIndex (in self). - let mut index_map: HashMap = HashMap::new(); - let replacement_nodes = r - .replacement - .children(r.replacement.root()) - .collect::>(); - // number of replacement nodes including Input and Output: - let replacement_sz = replacement_nodes.len(); - // slice of nodes omitting Input and Output: - let replacement_inner_nodes = &replacement_nodes[1..replacement_sz - 1]; - for &node in replacement_inner_nodes { - // Check there are no const inputs. - if !r - .replacement - .get_optype(node) - .signature() - .const_input - .is_empty() - { - return Err(SimpleReplacementError::InvalidReplacementNode()); - } - } - let self_input_node_index = self.hierarchy.first(r.parent.index).unwrap(); - let replacement_output_node = *replacement_nodes.last().unwrap(); - for &node in replacement_inner_nodes { - // Add the nodes. - let op: &OpType = r.replacement.get_optype(node); - let sig = op.signature(); - let new_node_index = self.graph.add_node(sig.input.len(), sig.output.len()); - self.op_types[new_node_index] = op.clone(); - // Make r.parent the parent - self.hierarchy - .insert_after(new_node_index, self_input_node_index) - .ok(); - index_map.insert(node.index, new_node_index); - } - // Add edges between all newly added nodes matching those in replacement. - // TODO This will probably change when implicit copies are implemented. - for &node in replacement_inner_nodes { - let new_node_index = index_map.get(&node.index).unwrap(); - for node_successor in r.replacement.output_neighbours(node) { - if r.replacement.get_optype(node_successor).tag() != OpTag::Output { - let new_node_successor_index = index_map.get(&node_successor.index).unwrap(); - for connection in r - .replacement - .graph - .get_connections(node.index, node_successor.index) - { - let src_offset = r - .replacement - .graph - .port_offset(connection.0) - .unwrap() - .index(); - let tgt_offset = r - .replacement - .graph - .port_offset(connection.1) - .unwrap() - .index(); - self.graph - .link_nodes( - *new_node_index, - src_offset, - *new_node_successor_index, - tgt_offset, - ) - .ok(); - } - } - } - } - // 3.2. For each p = r.nu_inp[q] such that q is not an Output port, add an edge from the - // predecessor of p to (the new copy of) q. - for ((rep_inp_node, rep_inp_port), (rem_inp_node, rem_inp_port)) in &r.nu_inp { - if r.replacement.get_optype(*rep_inp_node).tag() != OpTag::Output { - let new_inp_node_index = index_map.get(&rep_inp_node.index).unwrap(); - // add edge from predecessor of (s_inp_node, s_inp_port) to (new_inp_node, n_inp_port) - let rem_inp_port_index = self - .graph - .port_index(rem_inp_node.index, rem_inp_port.offset) - .unwrap(); - let rem_inp_predecessor_port_index = - self.graph.port_link(rem_inp_port_index).unwrap().port(); - let new_inp_port_index = self - .graph - .port_index(*new_inp_node_index, rep_inp_port.offset) - .unwrap(); - self.graph.unlink_port(rem_inp_predecessor_port_index); - self.graph - .link_ports(rem_inp_predecessor_port_index, new_inp_port_index) - .ok(); - } - } - // 3.3. For each q = r.nu_out[p] such that the predecessor of q is not an Input port, add an - // edge from (the new copy of) the predecessor of q to p. - for ((rem_out_node, rem_out_port), rep_out_port) in &r.nu_out { - let rem_out_port_index = self - .graph - .port_index(rem_out_node.index, rem_out_port.offset) - .unwrap(); - let rep_out_port_index = r - .replacement - .graph - .port_index(replacement_output_node.index, rep_out_port.offset) - .unwrap(); - let rep_out_predecessor_port_index = - r.replacement.graph.port_link(rep_out_port_index).unwrap(); - let rep_out_predecessor_node_index = r - .replacement - .graph - .port_node(rep_out_predecessor_port_index) - .unwrap(); - if r.replacement - .get_optype(rep_out_predecessor_node_index.into()) - .tag() - != OpTag::Input - { - let rep_out_predecessor_port_offset = r - .replacement - .graph - .port_offset(rep_out_predecessor_port_index) - .unwrap(); - let new_out_node_index = index_map.get(&rep_out_predecessor_node_index).unwrap(); - let new_out_port_index = self - .graph - .port_index(*new_out_node_index, rep_out_predecessor_port_offset) - .unwrap(); - self.graph.unlink_port(rem_out_port_index); - self.graph - .link_ports(new_out_port_index, rem_out_port_index) - .ok(); - } - } - // 3.4. For each q = r.nu_out[p1], p0 = r.nu_inp[q], add an edge from the predecessor of p0 - // to p1. - for ((rem_out_node, rem_out_port), &rep_out_port) in &r.nu_out { - let rem_inp_nodeport = r.nu_inp.get(&(replacement_output_node, rep_out_port)); - if let Some((rem_inp_node, rem_inp_port)) = rem_inp_nodeport { - // add edge from predecessor of (rem_inp_node, rem_inp_port) to (rem_out_node, rem_out_port): - let rem_inp_port_index = self - .graph - .port_index(rem_inp_node.index, rem_inp_port.offset) - .unwrap(); - let rem_inp_predecessor_port_index = - self.graph.port_link(rem_inp_port_index).unwrap().port(); - let rem_out_port_index = self - .graph - .port_index(rem_out_node.index, rem_out_port.offset) - .unwrap(); - self.graph.unlink_port(rem_inp_port_index); - self.graph.unlink_port(rem_out_port_index); - self.graph - .link_ports(rem_inp_predecessor_port_index, rem_out_port_index) - .ok(); - } - } - // 3.5. Remove all nodes in r.removal and edges between them. - for node in &r.removal { - self.graph.remove_node(node.index); - self.hierarchy.remove(node.index); - } - Ok(()) - } - /// Applies a rewrite to the graph. pub fn apply_rewrite(&mut self, rw: impl Rewrite) -> Result<(), E> { rw.apply(self) diff --git a/src/hugr/rewrite.rs b/src/hugr/rewrite.rs index 8455facff..9df71ffdd 100644 --- a/src/hugr/rewrite.rs +++ b/src/hugr/rewrite.rs @@ -1,11 +1,13 @@ -//! Replace operations on the HUGR. +//! Rewrite operations on the HUGR - replacement, outlining, etc. #[allow(clippy::module_inception)] // TODO: Rename? pub mod replace; +pub mod simple_replace; use std::mem; use crate::Hugr; pub use replace::{OpenHugr, Replace, ReplaceError}; +pub use simple_replace::{SimpleReplacement, SimpleReplacementError}; /// An operation that can be applied to mutate a Hugr pub trait Rewrite { diff --git a/src/replacement/simple_replace.rs b/src/hugr/rewrite/simple_replace.rs similarity index 59% rename from src/replacement/simple_replace.rs rename to src/hugr/rewrite/simple_replace.rs index 5328a7665..29d80db02 100644 --- a/src/replacement/simple_replace.rs +++ b/src/hugr/rewrite/simple_replace.rs @@ -2,7 +2,14 @@ use std::collections::{HashMap, HashSet}; -use crate::{hugr::Node, Hugr, Port}; +use portgraph::NodeIndex; + +use crate::hugr::HugrView; +use crate::{ + hugr::{Node, Rewrite}, + ops::{tag::OpTag, OpTrait, OpType}, + Hugr, Port, +}; use thiserror::Error; /// Specification of a simple replacement operation. @@ -41,6 +48,198 @@ impl SimpleReplacement { } } +impl Rewrite for SimpleReplacement { + const UNCHANGED_ON_FAILURE: bool = true; + + fn verify(&self, h: &Hugr) -> Result<(), SimpleReplacementError> { + unimplemented!() + } + + fn apply(self, h: &mut Hugr) -> Result<(), SimpleReplacementError> { + // 1. Check the parent node exists and is a DFG node. + if h.get_optype(self.parent).tag() != OpTag::Dfg { + return Err(SimpleReplacementError::InvalidParentNode()); + } + // 2. Check that all the to-be-removed nodes are children of it and are leaves. + for node in &self.removal { + if h.hierarchy.parent(node.index) != Some(self.parent.index) + || h.hierarchy.has_children(node.index) + { + return Err(SimpleReplacementError::InvalidRemovedNode()); + } + } + // 3. Do the replacement. + // 3.1. Add copies of all replacement nodes and edges to h. Exclude Input/Output nodes. + // Create map from old NodeIndex (in self.replacement) to new NodeIndex (in self). + let mut index_map: HashMap = HashMap::new(); + let replacement_nodes = self + .replacement + .children(self.replacement.root()) + .collect::>(); + // number of replacement nodes including Input and Output: + let replacement_sz = replacement_nodes.len(); + // slice of nodes omitting Input and Output: + let replacement_inner_nodes = &replacement_nodes[1..replacement_sz - 1]; + for &node in replacement_inner_nodes { + // Check there are no const inputs. + if !self + .replacement + .get_optype(node) + .signature() + .const_input + .is_empty() + { + return Err(SimpleReplacementError::InvalidReplacementNode()); + } + } + let self_input_node_index = h.hierarchy.first(self.parent.index).unwrap(); + let replacement_output_node = *replacement_nodes.last().unwrap(); + for &node in replacement_inner_nodes { + // Add the nodes. + let op: &OpType = self.replacement.get_optype(node); + let sig = op.signature(); + let new_node_index = h.graph.add_node(sig.input.len(), sig.output.len()); + h.op_types[new_node_index] = op.clone(); + // Make self.parent the parent + h.hierarchy + .insert_after(new_node_index, self_input_node_index) + .ok(); + index_map.insert(node.index, new_node_index); + } + // Add edges between all newly added nodes matching those in replacement. + // TODO This will probably change when implicit copies are implemented. + for &node in replacement_inner_nodes { + let new_node_index = index_map.get(&node.index).unwrap(); + for node_successor in self.replacement.output_neighbours(node) { + if self.replacement.get_optype(node_successor).tag() != OpTag::Output { + let new_node_successor_index = index_map.get(&node_successor.index).unwrap(); + for connection in self + .replacement + .graph + .get_connections(node.index, node_successor.index) + { + let src_offset = self + .replacement + .graph + .port_offset(connection.0) + .unwrap() + .index(); + let tgt_offset = self + .replacement + .graph + .port_offset(connection.1) + .unwrap() + .index(); + h.graph + .link_nodes( + *new_node_index, + src_offset, + *new_node_successor_index, + tgt_offset, + ) + .ok(); + } + } + } + } + // 3.2. For each p = self.nu_inp[q] such that q is not an Output port, add an edge from the + // predecessor of p to (the new copy of) q. + for ((rep_inp_node, rep_inp_port), (rem_inp_node, rem_inp_port)) in &self.nu_inp { + if self.replacement.get_optype(*rep_inp_node).tag() != OpTag::Output { + let new_inp_node_index = index_map.get(&rep_inp_node.index).unwrap(); + // add edge from predecessor of (s_inp_node, s_inp_port) to (new_inp_node, n_inp_port) + let rem_inp_port_index = h + .graph + .port_index(rem_inp_node.index, rem_inp_port.offset) + .unwrap(); + let rem_inp_predecessor_port_index = + h.graph.port_link(rem_inp_port_index).unwrap().port(); + let new_inp_port_index = h + .graph + .port_index(*new_inp_node_index, rep_inp_port.offset) + .unwrap(); + h.graph.unlink_port(rem_inp_predecessor_port_index); + h.graph + .link_ports(rem_inp_predecessor_port_index, new_inp_port_index) + .ok(); + } + } + // 3.3. For each q = self.nu_out[p] such that the predecessor of q is not an Input port, add an + // edge from (the new copy of) the predecessor of q to p. + for ((rem_out_node, rem_out_port), rep_out_port) in &self.nu_out { + let rem_out_port_index = h + .graph + .port_index(rem_out_node.index, rem_out_port.offset) + .unwrap(); + let rep_out_port_index = self + .replacement + .graph + .port_index(replacement_output_node.index, rep_out_port.offset) + .unwrap(); + let rep_out_predecessor_port_index = self + .replacement + .graph + .port_link(rep_out_port_index) + .unwrap(); + let rep_out_predecessor_node_index = self + .replacement + .graph + .port_node(rep_out_predecessor_port_index) + .unwrap(); + if self + .replacement + .get_optype(rep_out_predecessor_node_index.into()) + .tag() + != OpTag::Input + { + let rep_out_predecessor_port_offset = self + .replacement + .graph + .port_offset(rep_out_predecessor_port_index) + .unwrap(); + let new_out_node_index = index_map.get(&rep_out_predecessor_node_index).unwrap(); + let new_out_port_index = h + .graph + .port_index(*new_out_node_index, rep_out_predecessor_port_offset) + .unwrap(); + h.graph.unlink_port(rem_out_port_index); + h.graph + .link_ports(new_out_port_index, rem_out_port_index) + .ok(); + } + } + // 3.4. For each q = self.nu_out[p1], p0 = self.nu_inp[q], add an edge from the predecessor of p0 + // to p1. + for ((rem_out_node, rem_out_port), &rep_out_port) in &self.nu_out { + let rem_inp_nodeport = self.nu_inp.get(&(replacement_output_node, rep_out_port)); + if let Some((rem_inp_node, rem_inp_port)) = rem_inp_nodeport { + // add edge from predecessor of (rem_inp_node, rem_inp_port) to (rem_out_node, rem_out_port): + let rem_inp_port_index = h + .graph + .port_index(rem_inp_node.index, rem_inp_port.offset) + .unwrap(); + let rem_inp_predecessor_port_index = + h.graph.port_link(rem_inp_port_index).unwrap().port(); + let rem_out_port_index = h + .graph + .port_index(rem_out_node.index, rem_out_port.offset) + .unwrap(); + h.graph.unlink_port(rem_inp_port_index); + h.graph.unlink_port(rem_out_port_index); + h.graph + .link_ports(rem_inp_predecessor_port_index, rem_out_port_index) + .ok(); + } + } + // 3.5. Remove all nodes in self.removal and edges between them. + for node in &self.removal { + h.graph.remove_node(node.index); + h.hierarchy.remove(node.index); + } + Ok(()) + } +} + /// Error from a [`SimpleReplacement`] operation. #[derive(Debug, Clone, Error, PartialEq, Eq)] pub enum SimpleReplacementError { @@ -240,7 +439,7 @@ mod test { nu_inp, nu_out, }; - h.apply_simple_replacement(r).ok(); + h.apply_rewrite(r).ok(); // Expect [DFG] to be replaced with: // ┌───┐┌───┐ // ┤ H ├┤ H ├──■── @@ -333,7 +532,7 @@ mod test { nu_inp, nu_out, }; - h.apply_simple_replacement(r).ok(); + h.apply_rewrite(r).ok(); // Expect [DFG] to be replaced with: // ┌───┐┌───┐ // ┤ H ├┤ H ├ diff --git a/src/lib.rs b/src/lib.rs index 585c1af18..73e441c9f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -15,11 +15,11 @@ pub mod extensions; pub mod hugr; pub mod macros; pub mod ops; -pub mod replacement; pub mod resource; pub mod types; mod utils; -pub use crate::hugr::{Direction, Hugr, Node, Port, Replace, ReplaceError, Wire}; -pub use crate::replacement::SimpleReplacement; +pub use crate::hugr::{ + Direction, Hugr, Node, Port, Replace, ReplaceError, SimpleReplacement, Wire, +}; pub use crate::resource::Resource; From a83a93e19662bf8d86a28996439d0ee11f5d7bd1 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Fri, 9 Jun 2023 12:14:29 +0100 Subject: [PATCH 18/23] unused variable --- src/hugr/rewrite/simple_replace.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hugr/rewrite/simple_replace.rs b/src/hugr/rewrite/simple_replace.rs index 29d80db02..f6c6ad2b1 100644 --- a/src/hugr/rewrite/simple_replace.rs +++ b/src/hugr/rewrite/simple_replace.rs @@ -51,7 +51,7 @@ impl SimpleReplacement { impl Rewrite for SimpleReplacement { const UNCHANGED_ON_FAILURE: bool = true; - fn verify(&self, h: &Hugr) -> Result<(), SimpleReplacementError> { + fn verify(&self, _h: &Hugr) -> Result<(), SimpleReplacementError> { unimplemented!() } From 0291bba19d0dcc39323867cb977c004fee06ca63 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Fri, 9 Jun 2023 13:53:14 +0100 Subject: [PATCH 19/23] Drive-by: simple_replace.rs: change ".ok();"s to unwrap --- src/hugr/rewrite/simple_replace.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/hugr/rewrite/simple_replace.rs b/src/hugr/rewrite/simple_replace.rs index f6c6ad2b1..df5efba9f 100644 --- a/src/hugr/rewrite/simple_replace.rs +++ b/src/hugr/rewrite/simple_replace.rs @@ -103,7 +103,7 @@ impl Rewrite for SimpleReplacement { // Make self.parent the parent h.hierarchy .insert_after(new_node_index, self_input_node_index) - .ok(); + .unwrap(); index_map.insert(node.index, new_node_index); } // Add edges between all newly added nodes matching those in replacement. @@ -137,7 +137,7 @@ impl Rewrite for SimpleReplacement { *new_node_successor_index, tgt_offset, ) - .ok(); + .unwrap(); } } } @@ -161,7 +161,7 @@ impl Rewrite for SimpleReplacement { h.graph.unlink_port(rem_inp_predecessor_port_index); h.graph .link_ports(rem_inp_predecessor_port_index, new_inp_port_index) - .ok(); + .unwrap(); } } // 3.3. For each q = self.nu_out[p] such that the predecessor of q is not an Input port, add an @@ -205,7 +205,7 @@ impl Rewrite for SimpleReplacement { h.graph.unlink_port(rem_out_port_index); h.graph .link_ports(new_out_port_index, rem_out_port_index) - .ok(); + .unwrap(); } } // 3.4. For each q = self.nu_out[p1], p0 = self.nu_inp[q], add an edge from the predecessor of p0 @@ -228,7 +228,7 @@ impl Rewrite for SimpleReplacement { h.graph.unlink_port(rem_out_port_index); h.graph .link_ports(rem_inp_predecessor_port_index, rem_out_port_index) - .ok(); + .unwrap(); } } // 3.5. Remove all nodes in self.removal and edges between them. @@ -439,7 +439,7 @@ mod test { nu_inp, nu_out, }; - h.apply_rewrite(r).ok(); + h.apply_rewrite(r).unwrap(); // Expect [DFG] to be replaced with: // ┌───┐┌───┐ // ┤ H ├┤ H ├──■── @@ -532,7 +532,7 @@ mod test { nu_inp, nu_out, }; - h.apply_rewrite(r).ok(); + h.apply_rewrite(r).unwrap(); // Expect [DFG] to be replaced with: // ┌───┐┌───┐ // ┤ H ├┤ H ├ From 6070e20cf2abc229836d99c3b8815fa173a3a665 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 19 Jun 2023 12:39:37 +0100 Subject: [PATCH 20/23] Fix merge --- src/hugr.rs | 4 ++-- src/hugr/rewrite/simple_replace.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/hugr.rs b/src/hugr.rs index 8a7fbfe09..7db075641 100644 --- a/src/hugr.rs +++ b/src/hugr.rs @@ -16,11 +16,11 @@ pub use rewrite::{Replace, ReplaceError, Rewrite, SimpleReplacement, SimpleRepla use portgraph::dot::{DotFormat, EdgeStyle, NodeStyle, PortStyle}; use portgraph::multiportgraph::MultiPortGraph; -use portgraph::{Hierarchy, LinkView, NodeIndex, PortView, UnmanagedDenseMap}; +use portgraph::{Hierarchy, LinkView, PortView, UnmanagedDenseMap}; use thiserror::Error; pub use self::view::HugrView; -use crate::ops::{OpName, OpTrait, OpType}; +use crate::ops::{OpName, OpType}; use crate::types::EdgeKind; /// The Hugr data structure. diff --git a/src/hugr/rewrite/simple_replace.rs b/src/hugr/rewrite/simple_replace.rs index 0341f7315..dcc83a6b5 100644 --- a/src/hugr/rewrite/simple_replace.rs +++ b/src/hugr/rewrite/simple_replace.rs @@ -2,9 +2,9 @@ use std::collections::{HashMap, HashSet}; -use portgraph::NodeIndex; +use portgraph::{LinkView, NodeIndex, PortView}; -use crate::hugr::{HugrView, HugrMut}; +use crate::hugr::{HugrMut, HugrView}; use crate::{ hugr::{Node, Rewrite}, ops::{tag::OpTag, OpTrait, OpType}, From 4202f5e4bebda23dfb788b7ebd08480837dd6533 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 19 Jun 2023 12:39:46 +0100 Subject: [PATCH 21/23] Review comments --- src/hugr.rs | 2 +- src/hugr/rewrite.rs | 18 ++++++++++-------- src/hugr/rewrite/replace.rs | 3 ++- src/hugr/rewrite/simple_replace.rs | 3 ++- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/hugr.rs b/src/hugr.rs index 7db075641..f9fa9dd3b 100644 --- a/src/hugr.rs +++ b/src/hugr.rs @@ -80,7 +80,7 @@ pub struct Wire(Node, usize); /// Public API for HUGRs. impl Hugr { /// Applies a rewrite to the graph. - pub fn apply_rewrite(&mut self, rw: impl Rewrite) -> Result<(), E> { + pub fn apply_rewrite(&mut self, rw: impl Rewrite) -> Result<(), E> { rw.apply(self) } diff --git a/src/hugr/rewrite.rs b/src/hugr/rewrite.rs index 9df71ffdd..5037c0666 100644 --- a/src/hugr/rewrite.rs +++ b/src/hugr/rewrite.rs @@ -1,6 +1,5 @@ //! Rewrite operations on the HUGR - replacement, outlining, etc. -#[allow(clippy::module_inception)] // TODO: Rename? pub mod replace; pub mod simple_replace; use std::mem; @@ -10,7 +9,10 @@ pub use replace::{OpenHugr, Replace, ReplaceError}; pub use simple_replace::{SimpleReplacement, SimpleReplacementError}; /// An operation that can be applied to mutate a Hugr -pub trait Rewrite { +pub trait Rewrite { + /// The type of Error with which this Rewrite may fail + type Error: std::error::Error; + /// If `true`, [self.apply]'s of this rewrite guarantee that they do not mutate the Hugr when they return an Err. /// If `false`, there is no guarantee; the Hugr should be assumed invalid when Err is returned. const UNCHANGED_ON_FAILURE: bool; @@ -18,7 +20,7 @@ pub trait Rewrite { /// Checks whether the rewrite would succeed on the specified Hugr. /// If this call succeeds, [self.apply] should also succeed on the same `h` /// If this calls fails, [self.apply] would fail with the same error. - fn verify(&self, h: &Hugr) -> Result<(), E>; + fn verify(&self, h: &Hugr) -> Result<(), Self::Error>; /// Mutate the specified Hugr, or fail with an error. /// If [self.unchanged_on_failure] is true, then `h` must be unchanged if Err is returned. @@ -27,7 +29,7 @@ pub trait Rewrite { /// May panic if-and-only-if `h` would have failed [Hugr::validate]; that is, /// implementations may begin with `assert!(h.validate())`, with `debug_assert!(h.validate())` /// being preferred. - fn apply(self, h: &mut Hugr) -> Result<(), E>; + fn apply(self, h: &mut Hugr) -> Result<(), Self::Error>; } /// Wraps any rewrite into a transaction (i.e. that has no effect upon failure) @@ -37,18 +39,18 @@ pub struct Transactional { // Note we might like to constrain R to Rewrite but this // is not yet supported, https://github.com/rust-lang/rust/issues/92827 -impl> Rewrite for Transactional { +impl Rewrite for Transactional { + type Error = R :: Error; const UNCHANGED_ON_FAILURE: bool = true; - fn verify(&self, h: &Hugr) -> Result<(), E> { + fn verify(&self, h: &Hugr) -> Result<(), Self::Error> { self.underlying.verify(h) } - fn apply(self, h: &mut Hugr) -> Result<(), E> { + fn apply(self, h: &mut Hugr) -> Result<(), Self::Error> { if R::UNCHANGED_ON_FAILURE { return self.underlying.apply(h); } - // note that if underlying.may_fail_destructively(h) is false, we don't need a backup... let backup = h.clone(); let r = self.underlying.apply(h); if r.is_err() { diff --git a/src/hugr/rewrite/replace.rs b/src/hugr/rewrite/replace.rs index d75b2f70f..d8c591834 100644 --- a/src/hugr/rewrite/replace.rs +++ b/src/hugr/rewrite/replace.rs @@ -125,7 +125,8 @@ impl Replace { } } -impl Rewrite for Replace { +impl Rewrite for Replace { + type Error = ReplaceError; const UNCHANGED_ON_FAILURE: bool = false; /// Checks that the rewrite is valid. diff --git a/src/hugr/rewrite/simple_replace.rs b/src/hugr/rewrite/simple_replace.rs index dcc83a6b5..8b01c70b5 100644 --- a/src/hugr/rewrite/simple_replace.rs +++ b/src/hugr/rewrite/simple_replace.rs @@ -48,7 +48,8 @@ impl SimpleReplacement { } } -impl Rewrite for SimpleReplacement { +impl Rewrite for SimpleReplacement { + type Error = SimpleReplacementError; const UNCHANGED_ON_FAILURE: bool = true; fn verify(&self, _h: &Hugr) -> Result<(), SimpleReplacementError> { From 455339524790ab568c49921f9f21acc3ba302512 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 19 Jun 2023 12:40:49 +0100 Subject: [PATCH 22/23] todo -> unimplemented, the plan is not necessarily to do these --- src/hugr/rewrite/replace.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hugr/rewrite/replace.rs b/src/hugr/rewrite/replace.rs index d8c591834..fe25f6f52 100644 --- a/src/hugr/rewrite/replace.rs +++ b/src/hugr/rewrite/replace.rs @@ -117,11 +117,11 @@ impl Replace { } pub fn verify_convexity(&self) -> Result<(), ReplaceError> { - todo!() + unimplemented!() } pub fn verify_boundaries(&self) -> Result<(), ReplaceError> { - todo!() + unimplemented!() } } From 0201233ff1fbdc05353fd313ed4e3871f3d5640b Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 19 Jun 2023 12:43:05 +0100 Subject: [PATCH 23/23] fmt --- src/hugr.rs | 2 +- src/hugr/rewrite.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hugr.rs b/src/hugr.rs index f9fa9dd3b..2cb9ecf44 100644 --- a/src/hugr.rs +++ b/src/hugr.rs @@ -80,7 +80,7 @@ pub struct Wire(Node, usize); /// Public API for HUGRs. impl Hugr { /// Applies a rewrite to the graph. - pub fn apply_rewrite(&mut self, rw: impl Rewrite) -> Result<(), E> { + pub fn apply_rewrite(&mut self, rw: impl Rewrite) -> Result<(), E> { rw.apply(self) } diff --git a/src/hugr/rewrite.rs b/src/hugr/rewrite.rs index 5037c0666..dda3cde8d 100644 --- a/src/hugr/rewrite.rs +++ b/src/hugr/rewrite.rs @@ -40,7 +40,7 @@ pub struct Transactional { // Note we might like to constrain R to Rewrite but this // is not yet supported, https://github.com/rust-lang/rust/issues/92827 impl Rewrite for Transactional { - type Error = R :: Error; + type Error = R::Error; const UNCHANGED_ON_FAILURE: bool = true; fn verify(&self, h: &Hugr) -> Result<(), Self::Error> {