feat!: do not require an Order edge for each nonlocal Ext edge; compute on demand in scheduling_graph()

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,29 +851,21 @@ fn wire_up<T: Dataflow + ?Sized>(
             }
 
             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(),
                     dst,
                     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 {

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<LinkEndpoint: Eq, NodeIndexBase = u32, PortIndexBase = u32, PortOffsetBase = u32>
         + 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<Self::Node>;
 
-    /// 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<N>: 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<NodeMetadataMap> = 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<u32, u32, u32>> {
-        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::{

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<N: HugrNode> {
         to_offset: Port,
         ancestor_parent_op: Box<OpType>,
     },
-    /// 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})."

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()]));