diff --git a/hugr-core/src/builder/build_traits.rs b/hugr-core/src/builder/build_traits.rs index 63a7805bc7..9916128d5b 100644 --- a/hugr-core/src/builder/build_traits.rs +++ b/hugr-core/src/builder/build_traits.rs @@ -4,7 +4,7 @@ use crate::hugr::hugrmut::InsertionResult; use crate::hugr::linking::{HugrLinking, NameLinkingPolicy, NodeLinkingDirective}; use crate::hugr::views::HugrView; use crate::metadata::Metadata; -use crate::ops::{self, OpTag, OpTrait, OpType, Tag, TailLoop}; +use crate::ops::{self, OpType, Tag, TailLoop}; use crate::utils::collect_array; use crate::{Extension, IncomingPort, Node, OutgoingPort}; @@ -851,15 +851,14 @@ fn wire_up( } let src_parent = src_parent.expect("Node has no parent"); - let Some(src_sibling) = iter::successors(dst_parent, |&p| base.get_parent(p)) + if !iter::successors(dst_parent, |&p| base.get_parent(p)) .tuple_windows() - .find_map(|(ancestor, ancestor_parent)| { - (ancestor_parent == src_parent || + .any(|(_ancestor, ancestor_parent)| { + ancestor_parent == src_parent || // Dom edge - in CFGs - Some(ancestor_parent) == src_parent_parent) - .then_some(ancestor) + Some(ancestor_parent) == src_parent_parent }) - else { + { return Err(BuilderWiringError::NoRelationIntergraph { src, src_offset: src_port.into(), @@ -867,13 +866,6 @@ fn wire_up( dst_offset: dst_port.into(), }); }; - - if !OpTag::ControlFlowChild.is_superset(base.get_optype(src).tag()) - && !OpTag::ControlFlowChild.is_superset(base.get_optype(src_sibling).tag()) - { - // Add a state order constraint unless one of the nodes is a CFG BasicBlock - base.add_other_edge(src, src_sibling); - } } else if !typ.copyable() & base.linked_ports(src, src_port).next().is_some() { // Don't copy linear edges. return Err(BuilderWiringError::NoCopyLinear { diff --git a/hugr-core/src/hugr/internal.rs b/hugr-core/src/hugr/internal.rs index 9caa752fa7..e2b3f4403b 100644 --- a/hugr-core/src/hugr/internal.rs +++ b/hugr-core/src/hugr/internal.rs @@ -20,7 +20,9 @@ use crate::ops::handle::NodeHandle; /// Specifically, this trait provides access to the underlying portgraph /// view. pub trait HugrInternals { - /// The portgraph graph structure returned by [`HugrInternals::region_portgraph`]. + /// The portgraph graph structure used internally by [`scheduling_graph`]. + /// + /// [`scheduling_graph`]: HugrView::scheduling_graph type RegionPortgraph<'p>: LinkView + Clone + 'p @@ -31,27 +33,11 @@ pub trait HugrInternals { type Node: Copy + Ord + std::fmt::Debug + std::fmt::Display + std::hash::Hash; /// A mapping between HUGR nodes and portgraph nodes in the graph returned by - /// [`HugrInternals::region_portgraph`]. + /// [`scheduling_graph`]. + /// + /// [`scheduling_graph`]: HugrView::scheduling_graph type RegionPortgraphNodes: PortgraphNodeMap; - /// Returns a flat portgraph view of a region in the HUGR, and a mapping between - /// HUGR nodes and portgraph nodes in the graph. - // - // NOTE: Ideally here we would just return `Self::RegionPortgraph<'_>`, but - // when doing so we are unable to restrict the type to implement petgraph's - // traits over references (e.g. `&MyGraph : IntoNodeIdentifiers`, which is - // needed if we want to use petgraph's algorithms on the region graph). - // This won't be solvable until we do the big petgraph refactor -.- - // In the meantime, just wrap the portgraph in a `FlatRegion` as needed. - #[deprecated(note = "Use scheduling_graph instead", since = "0.27.0")] - fn region_portgraph( - &self, - parent: Self::Node, - ) -> ( - portgraph::view::FlatRegion<'_, Self::RegionPortgraph<'_>>, - Self::RegionPortgraphNodes, - ); - /// Returns a metadata entry associated with a node. /// /// # Panics @@ -60,8 +46,10 @@ pub trait HugrInternals { fn node_metadata_map(&self, node: Self::Node) -> &NodeMetadataMap; } -/// A map between hugr nodes and portgraph nodes in the graph returned by -/// [`HugrInternals::region_portgraph`]. +/// A map between hugr nodes and portgraph nodes in a [HugrInternals::RegionPortgraph] +/// or [`scheduling_graph`]. +/// +/// [`scheduling_graph`]: HugrView::scheduling_graph pub trait PortgraphNodeMap: Clone + Sized + std::fmt::Debug { /// Returns the portgraph index of a HUGR node in the associated region /// graph. @@ -120,20 +108,6 @@ impl HugrInternals for Hugr { type RegionPortgraphNodes = DefaultPGNodeMap; - #[inline] - fn region_portgraph( - &self, - parent: Self::Node, - ) -> ( - portgraph::view::FlatRegion<'_, Self::RegionPortgraph<'_>>, - Self::RegionPortgraphNodes, - ) { - let root = parent.into_portgraph(); - let region = - portgraph::view::FlatRegion::new_without_root(&self.graph, &self.hierarchy, root); - (region, DefaultPGNodeMap) - } - #[inline] fn node_metadata_map(&self, node: Self::Node) -> &NodeMetadataMap { static EMPTY: OnceLock = OnceLock::new(); @@ -393,23 +367,6 @@ impl HugrMutInternals for Hugr { } } -impl Hugr { - /// Consumes the HUGR and return a flat portgraph view of the region rooted - /// at `parent`. - #[inline] - #[deprecated(note = "Use scheduling_graph instead", since = "0.27.0")] - pub fn into_region_portgraph( - self, - parent: Node, - ) -> portgraph::view::FlatRegion<'static, MultiPortGraph> { - let root = parent.into_portgraph(); - let Self { - graph, hierarchy, .. - } = self; - portgraph::view::FlatRegion::new_without_root(graph, hierarchy, root) - } -} - #[cfg(test)] mod test { use crate::{ diff --git a/hugr-core/src/hugr/validate.rs b/hugr-core/src/hugr/validate.rs index 6648d4fd0c..b64f2e3fd3 100644 --- a/hugr-core/src/hugr/validate.rs +++ b/hugr-core/src/hugr/validate.rs @@ -486,19 +486,6 @@ impl<'a, H: HugrView> ValidationContext<'a, H> { { if ancestor_parent == from_parent { // External edge. - if !is_static { - // Must have an order edge. - self.hugr - .node_connections(from, ancestor) - .find(|&[p, _]| from_optype.port_kind(p) == Some(EdgeKind::StateOrder)) - .ok_or(InterGraphEdgeError::MissingOrderEdge { - from, - from_offset, - to, - to_offset, - to_ancestor: ancestor, - })?; - } return Ok(()); } else if Some(ancestor_parent) == from_parent_parent && !is_static { // Dominator edge @@ -807,17 +794,6 @@ pub enum InterGraphEdgeError { to_offset: Port, ancestor_parent_op: Box, }, - /// The sibling ancestors of the external inter-graph edge endpoints must be have an order edge between them. - #[error( - "Missing state order between the external inter-graph source {from} and the ancestor of the target {to_ancestor}. In an external inter-graph edge from {from} ({from_offset}) to {to} ({to_offset})." - )] - MissingOrderEdge { - from: N, - from_offset: Port, - to: N, - to_offset: Port, - to_ancestor: N, - }, /// The ancestors of an inter-graph edge are not related. #[error( "The ancestors of an inter-graph edge are not related. In an inter-graph edge from {from} ({from_offset}) to {to} ({to_offset})." diff --git a/hugr-core/src/hugr/validate/test.rs b/hugr-core/src/hugr/validate/test.rs index 7cec7d6e6d..8ec6299bb4 100644 --- a/hugr-core/src/hugr/validate/test.rs +++ b/hugr-core/src/hugr/validate/test.rs @@ -192,45 +192,6 @@ fn df_children_restrictions() { ); } -#[test] -fn test_ext_edge() { - let mut h = closed_dfg_root_hugr(Signature::new(vec![bool_t(), bool_t()], vec![bool_t()])); - let [input, output] = h.get_io(h.entrypoint()).unwrap(); - - // Nested DFG bool_t() -> bool_t() - let sub_dfg = h.add_node_with_parent( - h.entrypoint(), - ops::DFG { - signature: Signature::new_endo([bool_t()]), - }, - ); - // this Xor has its 2nd input unconnected - let sub_op = { - let sub_input = h.add_node_with_parent(sub_dfg, ops::Input::new(vec![bool_t()])); - let sub_output = h.add_node_with_parent(sub_dfg, ops::Output::new(vec![bool_t()])); - let sub_op = h.add_node_with_parent(sub_dfg, and_op()); - h.connect(sub_input, 0, sub_op, 0); - h.connect(sub_op, 0, sub_output, 0); - sub_op - }; - - h.connect(input, 0, sub_dfg, 0); - h.connect(sub_dfg, 0, output, 0); - - assert_matches!(h.validate(), Err(ValidationError::UnconnectedPort { .. })); - - h.connect(input, 1, sub_op, 1); - assert_matches!( - h.validate(), - Err(ValidationError::InterGraphEdgeError( - InterGraphEdgeError::MissingOrderEdge { .. } - )) - ); - //Order edge. This will need metadata indicating its purpose. - h.add_other_edge(input, sub_dfg); - h.validate().unwrap(); -} - #[test] fn test_local_const() { let mut h = closed_dfg_root_hugr(Signature::new_endo([bool_t()])); diff --git a/hugr-core/src/hugr/views.rs b/hugr-core/src/hugr/views.rs index 35f62c7993..780ac501a9 100644 --- a/hugr-core/src/hugr/views.rs +++ b/hugr-core/src/hugr/views.rs @@ -2,7 +2,6 @@ mod impls; mod nodes_iter; -pub mod petgraph; pub mod render; mod rerooted; mod root_checked; @@ -17,9 +16,6 @@ use serde::de::Deserialize; use std::borrow::Cow; use std::collections::HashMap; -#[deprecated(since = "0.26.0")] -#[expect(deprecated)] // Remove at same time -pub use self::petgraph::PetgraphWrapper; use self::render::MermaidFormatter; pub use nodes_iter::NodesIter; pub use rerooted::Rerooted; @@ -32,7 +28,7 @@ use portgraph::{LinkView, PortView}; use crate::core::HugrNode; use crate::extension::ExtensionRegistry; -use crate::hugr::internal::PortgraphNodeMap; +use crate::hugr::internal::{DefaultPGNodeMap, PortgraphNodeMap}; use crate::hugr::views::syn_edge::SynEdgeWrapper; use crate::metadata::{Metadata, RawMetadataValue}; use crate::ops::{OpParent, OpTag, OpTrait, OpType, handle::NodeHandle}; @@ -390,32 +386,9 @@ pub trait HugrView: HugrInternals { } } - /// Return a wrapper over the view that can be used in petgraph algorithms. - #[inline] - #[deprecated(since = "0.26.0", note = "Use HugrView::scheduling_graph instead.")] - #[expect(deprecated)] // Remove at same time as PetgraphWrapper - fn as_petgraph(&self) -> PetgraphWrapper<'_, Self> - where - Self: Sized, - { - PetgraphWrapper { hugr: self } - } - /// A view of a flat region, including ordering constraints from nonlocal edges, /// suitable for use with petgraph algorithms. - fn scheduling_graph(&self, parent: Self::Node) -> SchedulingGraph<'_, Self> { - #[expect(deprecated)] // Inline region_portgraph here when removing - let (region_view, region_nodes) = self.region_portgraph(parent); - let graph = SynEdgeWrapper { - region_view, - syn_edges: Vec::new(), - }; - SchedulingGraph { - graph, - node_map: region_nodes, - region_parent: parent, - } - } + fn scheduling_graph(&self, parent: Self::Node) -> SchedulingGraph<'_, Self>; /// Return the mermaid representation of the underlying hierarchical graph. /// @@ -607,15 +580,13 @@ impl<'a, V: HugrView + 'a> SchedulingGraph<'a, V> { self.node_map } + // Just ignore the syn edges. Use at own peril! fn portgraph_no_syn_edges( self, ) -> ( portgraph::view::FlatRegion<'a, V::RegionPortgraph<'a>>, V::RegionPortgraphNodes, ) { - // This may need to change when the SynEdgeWrapper actually has edges in it... - // or maybe we should keep the assert to prevent this being used any time it does. - assert!(self.graph.syn_edges.is_empty()); (self.graph.region_view, self.node_map) } @@ -852,6 +823,102 @@ impl HugrView for Hugr { } (extracted, DefaultNodeMap(inserted.node_map)) } + + fn scheduling_graph(&self, parent: Self::Node) -> SchedulingGraph<'_, Self> { + let root = parent.into_portgraph(); + let region_view = + portgraph::view::FlatRegion::new_without_root(&self.graph, &self.hierarchy, root); + let syn_edges = calc_syn_edges(self, parent); + let graph = SynEdgeWrapper { + region_view, + syn_edges, + }; + SchedulingGraph { + graph, + node_map: DefaultPGNodeMap, + region_parent: parent, + } + } +} + +fn calc_syn_edges>( + hugr: &H, + parent: H::Node, +) -> Vec<(portgraph::NodeIndex, portgraph::NodeIndex)> { + let mut syn_edges = Vec::new(); + if OpTag::DataflowParent.is_superset(hugr.get_optype(parent).tag()) { + let mut cache: HashMap = HashMap::new(); + fn find_sib_anc( + n: N, + hugr: &(impl HugrView + ?Sized), + cache: &mut HashMap, + parent: N, + ) -> Option { + // If we don't hit parent, it's a Dom edge, so ignore. + let p = hugr.get_parent(n)?; + if p == parent { + return Some(n); + } + match cache.get(&p) { + Some(&cached) => Some(cached), + None => { + // can't be borrowing cache during recursive call + let anc = find_sib_anc(p, hugr, cache, parent); + if let Some(anc) = anc { + cache.insert(p, anc); + } + anc + } + } + } + for child in hugr.children(parent) { + for (p, _) in hugr.out_value_types(child) { + for (tgt, _) in hugr.linked_inputs(child, p) { + if let Some(tgt_anc) = find_sib_anc(tgt, hugr, &mut cache, parent) + && tgt_anc != tgt + { + syn_edges.push((child.into_portgraph(), tgt_anc.into_portgraph())); + } + } + } + } + } + syn_edges +} + +/// Like [HugrView::scheduling_graph], but returns a [SchedulingGraph] that owns its own view of the region, +/// taken from the Hugr. +/// +/// The API is designed for `hugr-persistent` and allows to implement [HugrView] such that +/// [HugrInternals::RegionPortgraph] is not tied to the original view but rather an owned +/// temporary [Hugr]. +pub fn owned_scheduling_graph<'a, N: HugrNode, V>( + hugr: Hugr, + region_parent: N, + node_map: V::RegionPortgraphNodes, +) -> SchedulingGraph<'a, V> +where + V: for<'p> HugrView< + Node = N, + RegionPortgraph<'p> = portgraph::MultiPortGraph, + RegionPortgraphNodes = HashMap, + > + 'a, +{ + let parent = node_map[®ion_parent]; + let root = parent.into_portgraph(); + let syn_edges = calc_syn_edges(&hugr, parent); + let region_view = + portgraph::view::FlatRegion::new_without_root(hugr.graph, hugr.hierarchy, root); + + let graph = SynEdgeWrapper { + region_view, + syn_edges, + }; + SchedulingGraph { + graph, + node_map, + region_parent, + } } /// Trait implementing methods on port iterators. diff --git a/hugr-core/src/hugr/views/impls.rs b/hugr-core/src/hugr/views/impls.rs index 45b6e2a262..18ed4349fc 100644 --- a/hugr-core/src/hugr/views/impls.rs +++ b/hugr-core/src/hugr/views/impls.rs @@ -2,7 +2,7 @@ use std::{borrow::Cow, rc::Rc, sync::Arc}; -use super::HugrView; +use super::{HugrView, SchedulingGraph}; use crate::hugr::internal::{HugrInternals, HugrMutInternals}; use crate::hugr::{HugrMut, hugrmut::InsertForestResult}; @@ -11,8 +11,6 @@ macro_rules! hugr_internal_methods { ($arg:ident, $e:expr) => { delegate::delegate! { to ({let $arg=self; $e}) { - #[expect(deprecated)] // Remove delegate along with region_portgraph - fn region_portgraph(&self, parent: Self::Node) -> (portgraph::view::FlatRegion<'_, Self::RegionPortgraph<'_>>, Self::RegionPortgraphNodes); fn node_metadata_map(&self, node: Self::Node) -> &crate::hugr::NodeMetadataMap; } } @@ -70,6 +68,11 @@ macro_rules! hugr_view_methods { fn extract_hugr(&self, parent: Self::Node) -> (crate::Hugr, impl crate::hugr::views::ExtractionResult + 'static); } } + fn scheduling_graph(&self, parent: Self::Node) -> crate::hugr::views::SchedulingGraph<'_, Self> { + let $arg = self; + let SchedulingGraph {graph, node_map, region_parent} = $e.scheduling_graph(parent); + SchedulingGraph {graph, node_map, region_parent} + } } } diff --git a/hugr-core/src/hugr/views/petgraph.rs b/hugr-core/src/hugr/views/petgraph.rs deleted file mode 100644 index 6ab151bb3d..0000000000 --- a/hugr-core/src/hugr/views/petgraph.rs +++ /dev/null @@ -1,252 +0,0 @@ -//! Implementations of petgraph's traits for Hugr Region views. -#![allow(deprecated)] // Remove whole file when PetgraphWrapper is removed -use crate::core::HugrNode; -use crate::hugr::HugrView; -use crate::ops::OpType; -use crate::types::EdgeKind; -use crate::{NodeIndex, Port}; - -use petgraph::visit as pv; - -/// Wrapper for a `HugrView` that implements petgraph's traits. -/// -/// It can be used to apply petgraph's algorithms to a Hugr. -#[deprecated(since = "0.26.0")] -#[derive(Debug)] -pub struct PetgraphWrapper<'a, T> { - pub(crate) hugr: &'a T, -} - -impl Clone for PetgraphWrapper<'_, T> { - fn clone(&self) -> Self { - *self - } -} - -impl Copy for PetgraphWrapper<'_, T> {} - -impl<'a, T> From<&'a T> for PetgraphWrapper<'a, T> -where - T: HugrView, -{ - fn from(hugr: &'a T) -> Self { - Self { hugr } - } -} - -impl pv::GraphBase for PetgraphWrapper<'_, T> -where - T: HugrView, -{ - type NodeId = T::Node; - type EdgeId = ((T::Node, Port), (T::Node, Port)); -} - -impl pv::GraphProp for PetgraphWrapper<'_, T> -where - T: HugrView, -{ - type EdgeType = petgraph::Directed; -} - -impl pv::GraphRef for PetgraphWrapper<'_, T> where T: HugrView {} - -impl pv::NodeCount for PetgraphWrapper<'_, T> -where - T: HugrView, -{ - fn node_count(&self) -> usize { - HugrView::num_nodes(self.hugr) - } -} - -impl pv::NodeIndexable for PetgraphWrapper<'_, T> -where - T: HugrView, - // TODO: Define a trait for nodes that are equivalent to usizes, and implement it for `Node` - T::Node: NodeIndex + From, -{ - fn node_bound(&self) -> usize { - HugrView::num_nodes(self.hugr) - } - - fn to_index(&self, ix: Self::NodeId) -> usize { - ix.index() - } - - fn from_index(&self, ix: usize) -> Self::NodeId { - portgraph::NodeIndex::new(ix).into() - } -} - -impl pv::EdgeCount for PetgraphWrapper<'_, T> -where - T: HugrView, -{ - fn edge_count(&self) -> usize { - HugrView::num_edges(self.hugr) - } -} - -impl pv::Data for PetgraphWrapper<'_, T> -where - T: HugrView, -{ - type NodeWeight = OpType; - type EdgeWeight = EdgeKind; -} - -impl<'a, T> pv::IntoNodeReferences for PetgraphWrapper<'a, T> -where - T: HugrView, -{ - type NodeRef = HugrNodeRef<'a, T::Node>; - type NodeReferences = Box> + 'a>; - - fn node_references(self) -> Self::NodeReferences { - Box::new( - self.hugr - .nodes() - .map(|n| HugrNodeRef::from_node(n, self.hugr)), - ) - } -} - -impl<'a, T> pv::IntoNodeIdentifiers for PetgraphWrapper<'a, T> -where - T: HugrView, -{ - type NodeIdentifiers = Box + 'a>; - - fn node_identifiers(self) -> Self::NodeIdentifiers { - Box::new(self.hugr.nodes()) - } -} - -impl<'a, T> pv::IntoNeighbors for PetgraphWrapper<'a, T> -where - T: HugrView, -{ - type Neighbors = Box + 'a>; - - fn neighbors(self, n: Self::NodeId) -> Self::Neighbors { - Box::new(self.hugr.output_neighbours(n)) - } -} - -impl<'a, T> pv::IntoNeighborsDirected for PetgraphWrapper<'a, T> -where - T: HugrView, -{ - type NeighborsDirected = Box + 'a>; - - fn neighbors_directed( - self, - n: Self::NodeId, - d: petgraph::Direction, - ) -> Self::NeighborsDirected { - Box::new(self.hugr.neighbours(n, d.into())) - } -} - -impl pv::Visitable for PetgraphWrapper<'_, T> -where - T: HugrView, -{ - type Map = std::collections::HashSet; - - fn visit_map(&self) -> Self::Map { - std::collections::HashSet::new() - } - - fn reset_map(&self, map: &mut Self::Map) { - map.clear(); - } -} - -impl pv::GetAdjacencyMatrix for PetgraphWrapper<'_, T> -where - T: HugrView, -{ - type AdjMatrix = std::collections::HashSet<(Self::NodeId, Self::NodeId)>; - - fn adjacency_matrix(&self) -> Self::AdjMatrix { - let mut matrix = std::collections::HashSet::new(); - for node in self.hugr.nodes() { - for neighbour in self.hugr.output_neighbours(node) { - matrix.insert((node, neighbour)); - } - } - matrix - } - - fn is_adjacent(&self, matrix: &Self::AdjMatrix, a: Self::NodeId, b: Self::NodeId) -> bool { - matrix.contains(&(a, b)) - } -} - -/// Reference to a Hugr node and its associated `OpType`. -#[derive(Debug, Clone, Copy)] -pub struct HugrNodeRef<'a, N> { - node: N, - op: &'a OpType, -} - -impl<'a, N: HugrNode> HugrNodeRef<'a, N> { - pub(self) fn from_node(node: N, hugr: &'a impl HugrView) -> Self { - Self { - node, - op: hugr.get_optype(node), - } - } -} - -impl pv::NodeRef for HugrNodeRef<'_, N> { - type NodeId = N; - - type Weight = OpType; - - fn id(&self) -> Self::NodeId { - self.node - } - - fn weight(&self) -> &Self::Weight { - self.op - } -} - -#[cfg(test)] -mod test { - use petgraph::visit::{ - EdgeCount, GetAdjacencyMatrix, IntoNodeReferences, NodeCount, NodeIndexable, NodeRef, - }; - - use crate::HugrView; - use crate::hugr::views::tests::sample_hugr; - use crate::ops::handle::NodeHandle; - - use super::PetgraphWrapper; - - #[test] - fn test_petgraph_wrapper() { - let (hugr, cx1, cx2) = sample_hugr(); - let wrapper = PetgraphWrapper::from(&hugr); - - assert_eq!(wrapper.node_count(), 9); - assert_eq!(wrapper.node_bound(), 9); - assert_eq!(wrapper.edge_count(), 11); - - let cx1_index = cx1.node().into_portgraph().index(); - assert_eq!(wrapper.to_index(cx1.node()), cx1_index); - assert_eq!(wrapper.from_index(cx1_index), cx1.node()); - - let cx1_ref = wrapper - .node_references() - .find(|n| n.id() == cx1.node()) - .unwrap(); - assert_eq!(cx1_ref.weight(), hugr.get_optype(cx1.node())); - - let adj = wrapper.adjacency_matrix(); - assert!(wrapper.is_adjacent(&adj, cx1.node(), cx2.node())); - } -} diff --git a/hugr-core/src/hugr/views/rerooted.rs b/hugr-core/src/hugr/views/rerooted.rs index bc2a024cc7..08751105a6 100644 --- a/hugr-core/src/hugr/views/rerooted.rs +++ b/hugr-core/src/hugr/views/rerooted.rs @@ -4,7 +4,7 @@ use crate::hugr::internal::{HugrInternals, HugrMutInternals}; use crate::hugr::{HugrMut, hugrmut::InsertForestResult}; -use super::{HugrView, panic_invalid_node}; +use super::{HugrView, SchedulingGraph, panic_invalid_node}; /// A HUGR wrapper with a modified entrypoint node. /// @@ -110,6 +110,18 @@ impl HugrView for Rerooted { fn extract_hugr(&self, parent: Self::Node) -> (crate::Hugr, impl crate::hugr::views::ExtractionResult + 'static); } } + fn scheduling_graph(&self, parent: Self::Node) -> super::SchedulingGraph<'_, Self> { + let SchedulingGraph { + graph, + node_map, + region_parent, + } = self.hugr.scheduling_graph(parent); + SchedulingGraph { + graph, + node_map, + region_parent, + } + } } impl HugrMutInternals for Rerooted { diff --git a/hugr-core/src/hugr/views/sibling_subgraph.rs b/hugr-core/src/hugr/views/sibling_subgraph.rs index 704e5a2d5a..59ae0c7e2e 100644 --- a/hugr-core/src/hugr/views/sibling_subgraph.rs +++ b/hugr-core/src/hugr/views/sibling_subgraph.rs @@ -266,7 +266,7 @@ impl SiblingSubgraph { /// You MUST make sure that the boundary ports and nodes provided satisfy /// the SiblingSubgraph validity conditions described in /// [`SiblingSubgraph::try_new`] and which can be checked using - /// [`SiblingSubgraph::validate`]. + /// [`SiblingSubgraph::validate_with_checker`] or friends. /// /// See [`SiblingSubgraph::try_new`] for the full documentation. /// @@ -517,25 +517,6 @@ impl SiblingSubgraph { } } - /// Check the validity of the subgraph, as described in the docs of - /// [`SiblingSubgraph::try_new`]. - #[deprecated( - note = "Use `validate_with_checker`, `validate_default` or `validate_skip_convexity`", - since = "0.27.1" - )] - #[expect(deprecated)] // Remove with ValidationMode - pub fn validate<'h, H: HugrView>( - &self, - hugr: &'h H, - mode: ValidationMode<'_, 'h, H>, - ) -> Result<(), InvalidSubgraph> { - match mode { - ValidationMode::WithChecker(checker) => self.validate_with_checker(hugr, Some(checker)), - ValidationMode::CheckConvexity => self.validate_default(hugr), - ValidationMode::SkipConvexity => self.validate_skip_convexity(hugr), - } - } - /// Check the validity of the subgraph, as described in the docs of /// [`SiblingSubgraph::try_new`], using a new [`SchedGraphChecker`] for the convexity check. pub fn validate_default( @@ -829,23 +810,6 @@ impl SiblingSubgraph { } } -/// Specify the checks to perform for [`SiblingSubgraph::validate`]. -#[allow(deprecated)] // Remove enum along with TopoConvexChecker -#[deprecated( - note = "Call validate_with_checker or validate_default instead", - since = "0.27.1" -)] -#[derive(Default)] -pub enum ValidationMode<'t, 'h, H: HugrView> { - /// Check convexity with the given checker. - WithChecker(&'t TopoConvexChecker<'h, H>), - /// Construct a checker and check convexity. - #[default] - CheckConvexity, - /// Skip convexity check. - SkipConvexity, -} - fn make_pg_subgraph<'h, H: HugrView>( region: CheckerRegion<'h, H>, inputs: &IncomingPorts, @@ -1049,22 +1013,6 @@ fn check_parent<'a, N: HugrNode>( type CheckerRegion<'g, Base> = portgraph::view::FlatRegion<'g, ::RegionPortgraph<'g>>; -/// Precompute convexity information for a HUGR. -/// -/// This can be used when constructing multiple sibling subgraphs to speed up -/// convexity checking. -/// -/// This a good default choice for most convexity checking use cases. -#[deprecated( - note = "Use SchedGraphChecker or LineConvexChecker instead", - since = "0.27.1" -)] -pub type TopoConvexChecker<'g, Base> = PortgraphCheckerWithNodes< - 'g, - Base, - portgraph::algorithms::TopoConvexChecker>, ->; - /// Precompute convexity information for a HUGR. /// /// This can be used when constructing multiple sibling subgraphs to speed up @@ -1085,7 +1033,6 @@ pub type LineConvexChecker<'g, Base> = PortgraphCheckerWithNodes< /// /// This type is generic over the convexity checker used. If checking convexity /// for circuit-like graphs, use [`LineConvexChecker`]. Alternatively, use [SchedGraphChecker]. -/// [`TopoConvexChecker`]. #[derive(Clone)] pub struct PortgraphCheckerWithNodes<'g, Base: HugrView, Checker> { /// The base HUGR to check convexity on. @@ -1098,10 +1045,6 @@ pub struct PortgraphCheckerWithNodes<'g, Base: HugrView, Checker> { node_map: Base::RegionPortgraphNodes, } -#[deprecated(note = "Use PortgraphCheckerWithNodes instead", since = "0.27.1")] -/// Use [PortgraphCheckerWithNodes] -pub type ConvexChecker<'g, Base, Checker> = PortgraphCheckerWithNodes<'g, Base, Checker>; - impl<'g, Base, Checker> PortgraphCheckerWithNodes<'g, Base, Checker> where Base: HugrView, diff --git a/hugr-core/src/hugr/views/sibling_subgraph/tests.rs b/hugr-core/src/hugr/views/sibling_subgraph/tests.rs index 4a033f6d26..ede23c44df 100644 --- a/hugr-core/src/hugr/views/sibling_subgraph/tests.rs +++ b/hugr-core/src/hugr/views/sibling_subgraph/tests.rs @@ -794,7 +794,7 @@ fn test_nonlocal_edge_excluding_target() { // Sanity check - simple SSG without the nonlocal edge assert_eq!( h.output_neighbours(not_op.node()).collect_vec(), - vec![nested_not.node(), dfg.node()] + vec![nested_not.node()] ); let outer_not_inputs = vec![vec![(not_op.node(), IncomingPort::from(0))]]; let ss = SiblingSubgraph::try_new( @@ -803,15 +803,8 @@ fn test_nonlocal_edge_excluding_target() { &h, ) .unwrap(); - // Nodes include the DFG (by following Order edge) and Output (edge from DFG) - assert_eq!( - ss.nodes(), - &[ - h.get_io(h.entrypoint()).unwrap()[1], - not_op.node(), - dfg.node() - ] - ); + // Does not include the DFG as does not follow the Syn edge + assert_eq!(ss.nodes, &[not_op.node()]); ss.validate_default(&h).unwrap(); // We can't "not" follow the Order edge.... @@ -826,7 +819,12 @@ fn test_nonlocal_edge_excluding_target() { ], &h, ); - assert_matches!(ss2, Err(InvalidSubgraph::UnsupportedEdgeKind(_, _))); + assert_matches!( + ss2, + Err(InvalidSubgraph::InvalidBoundary( + InvalidSubgraphBoundary::DisconnectedBoundaryPort(_, _) + )) + ); // Now try to make an SSG with the outer Not and the DFG...this should not be possible ATM // (it would contain an edge to the inner Not, thus contain the inner Not, thus is not a sibling subgraph). diff --git a/hugr-core/src/hugr/views/tests.rs b/hugr-core/src/hugr/views/tests.rs index 1df56377e8..4ae7a8f455 100644 --- a/hugr-core/src/hugr/views/tests.rs +++ b/hugr-core/src/hugr/views/tests.rs @@ -223,10 +223,7 @@ fn test_syn_edge() { let [sub_dfg] = sub_dfg.finish_with_outputs([not]).unwrap().outputs_arr(); let h = outer.finish_hugr_with_outputs([sub_dfg]).unwrap(); - assert_eq!( - h.output_neighbours(inp.node()).collect_vec(), - [not.node(), sub_dfg.node()] - ); + assert_eq!(h.output_neighbours(inp.node()).collect_vec(), [not.node()]); // No sub_dfg let sg = h.scheduling_graph(h.entrypoint()); assert!( diff --git a/hugr-llvm/src/emit/ops/snapshots/hugr_llvm__emit__ops__cfg__test__nested@pre-mem2reg@llvm21.snap b/hugr-llvm/src/emit/ops/snapshots/hugr_llvm__emit__ops__cfg__test__nested@pre-mem2reg@llvm21.snap index ac3622fb8e..fab56b6525 100644 --- a/hugr-llvm/src/emit/ops/snapshots/hugr_llvm__emit__ops__cfg__test__nested@pre-mem2reg@llvm21.snap +++ b/hugr-llvm/src/emit/ops/snapshots/hugr_llvm__emit__ops__cfg__test__nested@pre-mem2reg@llvm21.snap @@ -19,8 +19,8 @@ alloca_block: %"06" = alloca i1, align 1 %"20_0" = alloca i1, align 1 %"25_0" = alloca i1, align 1 - %"39_0" = alloca i1, align 1 - %"44_0" = alloca i1, align 1 + %"35_0" = alloca i1, align 1 + %"40_0" = alloca i1, align 1 br label %entry_block entry_block: ; preds = %alloca_block @@ -41,33 +41,33 @@ entry_block: ; preds = %alloca_block br label %8 3: ; preds = %19 - store i1 true, ptr %"39_0", align 1 + store i1 true, ptr %"35_0", align 1 %"5_025" = load {}, ptr %"5_0", align 1 - %"39_026" = load i1, ptr %"39_0", align 1 + %"35_026" = load i1, ptr %"35_0", align 1 store {} %"5_025", ptr %"5_0", align 1 - store i1 %"39_026", ptr %"39_0", align 1 + store i1 %"35_026", ptr %"35_0", align 1 %"5_027" = load {}, ptr %"5_0", align 1 - %"39_028" = load i1, ptr %"39_0", align 1 + %"35_028" = load i1, ptr %"35_0", align 1 switch i1 false, label %4 [ ] 4: ; preds = %3 - store i1 %"39_028", ptr %"03", align 1 + store i1 %"35_028", ptr %"03", align 1 br label %7 5: ; preds = %20 - store i1 false, ptr %"44_0", align 1 + store i1 false, ptr %"40_0", align 1 %"5_029" = load {}, ptr %"5_0", align 1 - %"44_030" = load i1, ptr %"44_0", align 1 + %"40_030" = load i1, ptr %"40_0", align 1 store {} %"5_029", ptr %"5_0", align 1 - store i1 %"44_030", ptr %"44_0", align 1 + store i1 %"40_030", ptr %"40_0", align 1 %"5_031" = load {}, ptr %"5_0", align 1 - %"44_032" = load i1, ptr %"44_0", align 1 + %"40_032" = load i1, ptr %"40_0", align 1 switch i1 false, label %6 [ ] 6: ; preds = %5 - store i1 %"44_032", ptr %"03", align 1 + store i1 %"40_032", ptr %"03", align 1 br label %7 7: ; preds = %6, %4 diff --git a/hugr-persistent/src/parents_view.rs b/hugr-persistent/src/parents_view.rs index 7d7b37e4b7..c90bca5c31 100644 --- a/hugr-persistent/src/parents_view.rs +++ b/hugr-persistent/src/parents_view.rs @@ -44,16 +44,6 @@ impl HugrInternals for ParentsView<'_> { type RegionPortgraphNodes = HashMap; - fn region_portgraph( - &self, - _parent: Self::Node, - ) -> ( - portgraph::view::FlatRegion<'_, Self::RegionPortgraph<'_>>, - Self::RegionPortgraphNodes, - ) { - unimplemented!() - } - fn node_metadata_map(&self, node: Self::Node) -> &hugr::NodeMetadataMap { let PatchNode(commit_id, node) = node; self.hugrs @@ -218,4 +208,8 @@ impl HugrView for ParentsView<'_> { #[allow(unreachable_code)] (unimplemented!(), HashMap::new()) } + + fn scheduling_graph(&self, _parent: Self::Node) -> hugr::views::SchedulingGraph<'_, Self> { + unimplemented!() + } } diff --git a/hugr-persistent/src/trait_impls.rs b/hugr-persistent/src/trait_impls.rs index 7593c26f26..ea4a2468bc 100644 --- a/hugr-persistent/src/trait_impls.rs +++ b/hugr-persistent/src/trait_impls.rs @@ -12,7 +12,7 @@ use hugr_core::{ self, Patch, SimpleReplacementError, internal::HugrInternals, views::{ - ExtractionResult, + ExtractionResult, owned_scheduling_graph, render::{MermaidFormatter, NodeLabel}, }, }, @@ -55,20 +55,6 @@ impl HugrInternals for PersistentHugr { type RegionPortgraphNodes = HashMap; - fn region_portgraph( - &self, - parent: Self::Node, - ) -> ( - portgraph::view::FlatRegion<'_, Self::RegionPortgraph<'_>>, - Self::RegionPortgraphNodes, - ) { - // TODO: this is currently not very efficient (see #2248) - let (hugr, node_map) = self.apply_all(); - let parent = node_map[&parent]; - #[expect(deprecated)] // Remove region_portgraph at same time - (hugr.into_region_portgraph(parent), node_map) - } - fn node_metadata_map(&self, PatchNode(commit_id, node): Self::Node) -> &hugr::NodeMetadataMap { let cm = self.get_commit(commit_id); cm.node_metadata_map(node) @@ -343,6 +329,12 @@ impl HugrView for PersistentHugr { (extracted_hugr, node_map) } + + fn scheduling_graph(&self, parent: Self::Node) -> hugr::views::SchedulingGraph<'_, Self> { + // TODO: this is currently not very efficient (https://github.com/Quantinuum/hugr/issues/2248) + let (hugr, node_map) = self.apply_all(); + owned_scheduling_graph(hugr, parent, node_map) + } } /// An iterator over nodes in a `PersistentHugr` that filters out invalid nodes. diff --git a/hugr-py/src/hugr/build/dfg.py b/hugr-py/src/hugr/build/dfg.py index fff8688e50..f72ef78776 100644 --- a/hugr-py/src/hugr/build/dfg.py +++ b/hugr-py/src/hugr/build/dfg.py @@ -678,8 +678,6 @@ def _wire_up_port(self, node: Node, offset: PortOffset, p: Wire) -> tys.Type: node_ancestor = _ancestral_sibling(self.hugr, src.node, node) if node_ancestor is None: raise NoSiblingAncestor(src.node.idx, node.idx) - if node_ancestor != node: - self.add_state_order(src.node, node_ancestor) self.hugr.add_link(src, node.inp(offset)) return self._get_dataflow_type(src) diff --git a/hugr-py/tests/__snapshots__/test_hugr_build.ambr b/hugr-py/tests/__snapshots__/test_hugr_build.ambr index 669eb24633..24ce00c2b5 100644 --- a/hugr-py/tests/__snapshots__/test_hugr_build.ambr +++ b/hugr-py/tests/__snapshots__/test_hugr_build.ambr @@ -523,7 +523,6 @@ } 2:"out.0" -> 4:"in.0" [arrowhead=none arrowsize=1.0 color="#1CADE4" fontcolor=black fontname=monospace fontsize=9 penwidth=1.5 xlabel=Bool] 2:"out.1" -> 4:"in.1" [arrowhead=none arrowsize=1.0 color="#1CADE4" fontcolor=black fontname=monospace fontsize=9 penwidth=1.5 xlabel=Bool] - 5:"out.-1" -> 7:"in.-1" [arrowhead=none arrowsize=1.0 color=black fontcolor=black fontname=monospace fontsize=9 penwidth=1.5 xlabel=""] 5:"out.0" -> 10:"in.0" [arrowhead=none arrowsize=1.0 color="#1CADE4" fontcolor=black fontname=monospace fontsize=9 penwidth=1.5 xlabel=Bool] 10:"out.0" -> 9:"in.0" [arrowhead=none arrowsize=1.0 color="#1CADE4" fontcolor=black fontname=monospace fontsize=9 penwidth=1.5 xlabel=Bool] 7:"out.0" -> 6:"in.0" [arrowhead=none arrowsize=1.0 color="#1CADE4" fontcolor=black fontname=monospace fontsize=9 penwidth=1.5 xlabel=Bool] diff --git a/hugr-py/tests/test_hugr_build.py b/hugr-py/tests/test_hugr_build.py index 8665fbef3a..2749f0a7fc 100644 --- a/hugr-py/tests/test_hugr_build.py +++ b/hugr-py/tests/test_hugr_build.py @@ -8,7 +8,7 @@ from hugr.build.dfg import Dfg, Function, _ancestral_sibling from hugr.build.function import Module from hugr.hugr import Hugr -from hugr.hugr.node_port import Node, _SubPort +from hugr.hugr.node_port import Node from hugr.ops import NoConcreteFunc from hugr.package import Package from hugr.std.int import INT_T, DivMod, IntVal @@ -186,6 +186,8 @@ def test_build_nested(snapshot): def test_build_inter_graph(snapshot): + # Possibly a bit redundant now, really we're just testing that we *don't* do + # anything special anymore, following https://github.com/Quantinuum/hugr/pull/2951. h = Dfg(tys.Bool, tys.Bool) (a, b) = h.inputs() with h.add_nested() as nested: @@ -196,10 +198,9 @@ def test_build_inter_graph(snapshot): validate(h.hugr, snap=snapshot) - assert _SubPort(h.input_node.out(-1)) in h.hugr._links - assert h.hugr.num_outgoing(h.input_node) == 3 - assert len(list(h.hugr.outgoing_order_links(h.input_node))) == 1 - assert len(list(h.hugr.incoming_order_links(nested))) == 1 + assert h.hugr.num_outgoing(h.input_node) == 2 + assert len(list(h.hugr.outgoing_order_links(h.input_node))) == 0 + assert len(list(h.hugr.incoming_order_links(nested))) == 0 assert len(list(h.hugr.incoming_order_links(h.output_node))) == 0 diff --git a/specification/hugr.md b/specification/hugr.md index d330f3e37f..51e1821e48 100644 --- a/specification/hugr.md +++ b/specification/hugr.md @@ -300,7 +300,11 @@ flowchart In a dataflow graph, the evaluation semantics are simple: all nodes in the graph are necessarily evaluated, in some order (perhaps parallel) -respecting the Dataflow edges. The following operations are used to +respecting the Dataflow and Order edges. This may include interleaving or +overlapping evaluation of siblings and descendants as long as said edges +are respected. + +The following operations are used to express control flow, i.e. conditional or repeated evaluation. #### `Conditional` nodes @@ -407,7 +411,7 @@ flowchart TB (control-flow-graphs)= #### Control Flow Graphs -When Conditional and `TailLoop` are not sufficient, the HUGR allows +When `Conditional` and `TailLoop` are not sufficient, the HUGR allows arbitrarily-complex (even irreducible) control flow via an explicit `CFG` node: a dataflow node defined by a child control sibling graph. This sibling graph contains `BasicBlock` nodes (and [scoped definitions](#scoped-definitions)), @@ -545,9 +549,12 @@ graph: cycles. The common parent is a CFG-node. **Dataflow Sibling Graph (DSG)**: nodes are operations, `CFG`, -`Conditional`, `TailLoop` and `DFG` nodes; edges are `Value`, `Order` and `Static`, and must be acyclic. +`Conditional`, `TailLoop` and `DFG` nodes; edges are `Value`, `Order` and `Static`. +The edges, along with the constraints that the source of any nonlocal edge must be before +the sibling that is ancestor of its target, must be acyclic. (Thus a valid ordering of operations can be achieved by topologically sorting the -nodes.) +nodes respecting the edges and this constraint.) + There is a unique Input node and Output node. The common parent may be a `FuncDefn`, `TailLoop`, `DFG`, `Case` or `DFB` node. @@ -578,19 +585,7 @@ parent(n2) when the edge's locality is: Each of these localities have additional constraints as follows: 1. For Ext edges, we require parent(n1) == - parenti(n2) for some i\>1, *and* for Value edges only there must be a order edge from n1 to - parenti-1(n2). - - The order edge records the - ordering requirement that results, i.e. it must be possible to - execute the entire n1 node before executing - parenti-1(n2). (Further recall that - order+value edges together must be acyclic). We record the - relationship between the Value edge and the - corresponding order edge via metadata on each edge. - - For Static edges this order edge is not required since the source is - guaranteed to causally precede the target. + parenti(n2) for some i\>1. 2. For Dom edges, we must have that parent2(n1) == parenti(n2) is a CFG-node, for some i\>1,