Skip to content

Commit

Permalink
fix: Allocate ports on root nodes (#1585)
Browse files Browse the repository at this point in the history
Closes #1584 

Not having ports in the root nodes caused problems when querying
information about them, as shown in #1584.
This PR changes the behaviour to instead allocate the ports, and
validate that they are always disconnected.
This should help make the API more homogeneous.
  • Loading branch information
aborgna-q authored Oct 17, 2024
1 parent 2412599 commit a66c4b9
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 56 deletions.
4 changes: 1 addition & 3 deletions hugr-core/src/hugr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,7 @@ impl Hugr {
let mut graph = MultiPortGraph::with_capacity(nodes, ports);
let hierarchy = Hierarchy::new();
let mut op_types = UnmanagedDenseMap::with_capacity(nodes);
let root = graph.add_node(0, 0);
// TODO: These extensions should be open in principle, but lets wait
// until extensions can be inferred for open sets until changing this
let root = graph.add_node(root_node.input_count(), root_node.output_count());
op_types[root] = root_node;

Self {
Expand Down
48 changes: 26 additions & 22 deletions hugr-core/src/hugr/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,11 +166,25 @@ impl<'a, 'b> ValidationContext<'a, 'b> {
))?;
}
// The Hugr can have only one root node.

for dir in Direction::BOTH {
// Check that we have the correct amount of ports and edges.
let num_ports = self.hugr.graph.num_ports(node.pg_index(), dir);
if num_ports != op_type.port_count(dir) {
return Err(ValidationError::WrongNumberOfPorts {
node,
optype: op_type.clone(),
actual: num_ports,
expected: op_type.port_count(dir),
dir,
});
}
}

if node == self.hugr.root() {
// The root node has no edges.
if self.hugr.graph.num_outputs(node.pg_index())
+ self.hugr.graph.num_inputs(node.pg_index())
!= 0
// The root node cannot have connected edges
if self.hugr.all_linked_inputs(node).next().is_some()
|| self.hugr.all_linked_outputs(node).next().is_some()
{
return Err(ValidationError::RootWithEdges { node });
}
Expand All @@ -190,20 +204,6 @@ impl<'a, 'b> ValidationContext<'a, 'b> {
allowed_children,
});
}

for dir in Direction::BOTH {
// Check that we have the correct amount of ports and edges.
let num_ports = self.hugr.graph.num_ports(node.pg_index(), dir);
if num_ports != op_type.port_count(dir) {
return Err(ValidationError::WrongNumberOfPorts {
node,
optype: op_type.clone(),
actual: num_ports,
expected: op_type.port_count(dir),
dir,
});
}
}
}

// Thirdly that the node has correct children
Expand Down Expand Up @@ -602,10 +602,14 @@ impl<'a, 'b> ValidationContext<'a, 'b> {
}

// Check port connections.
for dir in Direction::BOTH {
for (i, port_index) in self.hugr.graph.ports(node.pg_index(), dir).enumerate() {
let port = Port::new(dir, i);
self.validate_port(node, port, port_index, op_type, var_decls)?;
//
// Root nodes are ignored, as they cannot have connected edges.
if node != self.hugr.root() {
for dir in Direction::BOTH {
for (i, port_index) in self.hugr.graph.ports(node.pg_index(), dir).enumerate() {
let port = Port::new(dir, i);
self.validate_port(node, port, port_index, op_type, var_decls)?;
}
}
}

Expand Down
58 changes: 34 additions & 24 deletions hugr-core/src/hugr/validate/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ use crate::types::{
CustomType, FuncValueType, PolyFuncType, PolyFuncTypeRV, Signature, Type, TypeBound, TypeRV,
TypeRow,
};
use crate::{const_extension_ids, test_file, type_row, Direction, IncomingPort, Node};
use crate::{
const_extension_ids, test_file, type_row, Direction, IncomingPort, Node, OutgoingPort,
};

const NAT: Type = crate::extension::prelude::USIZE_T;

Expand Down Expand Up @@ -68,36 +70,44 @@ fn add_df_children(b: &mut Hugr, parent: Node, copies: usize) -> (Node, Node, No

#[test]
fn invalid_root() {
let declare_op: OpType = ops::FuncDecl {
name: "main".into(),
signature: Default::default(),
}
.into();

let mut b = Hugr::default();
let mut b = Hugr::new(LogicOp::Not);
let root = b.root();
assert_eq!(b.validate(&EMPTY_REG), Ok(()));
assert_eq!(b.validate(&PRELUDE_REGISTRY), Ok(()));

// Change the number of ports in the root
b.set_num_ports(root, 1, 0);
assert_matches!(
b.validate(&PRELUDE_REGISTRY),
Err(ValidationError::WrongNumberOfPorts { node, .. }) => assert_eq!(node, root)
);
b.set_num_ports(root, 2, 2);

// Connect it to itself
b.connect(root, 0, root, 0);
assert_matches!(
b.validate(&PRELUDE_REGISTRY),
Err(ValidationError::RootWithEdges { node, .. }) => assert_eq!(node, root)
);
b.disconnect(root, OutgoingPort::from(0));

// Add another hierarchy root
let other = b.add_node(ops::Module::new().into());
let module = b.add_node(ops::Module::new().into());
assert_matches!(
b.validate(&EMPTY_REG),
Err(ValidationError::NoParent { node }) => assert_eq!(node, other)
b.validate(&PRELUDE_REGISTRY),
Err(ValidationError::NoParent { node }) => assert_eq!(node, module)
);
b.set_parent(other, root);
b.replace_op(other, declare_op).unwrap();
b.add_ports(other, Direction::Outgoing, 1);
assert_eq!(b.validate(&EMPTY_REG), Ok(()));

// Make the hugr root not a hierarchy root
{
let mut hugr = b.clone();
hugr.root = other.pg_index();
assert_matches!(
hugr.validate(&EMPTY_REG),
Err(ValidationError::RootNotRoot { node }) => assert_eq!(node, other)
);
}
b.set_parent(root, module);
assert_matches!(
b.validate(&PRELUDE_REGISTRY),
Err(ValidationError::RootNotRoot { node }) => assert_eq!(node, root)
);

// Fix the root
b.root = module.pg_index();
b.remove_node(root);
assert_eq!(b.validate(&PRELUDE_REGISTRY), Ok(()));
}

#[test]
Expand Down
3 changes: 1 addition & 2 deletions hugr-core/src/hugr/views.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use itertools::{Itertools, MapInto};
use portgraph::render::{DotFormat, MermaidFormat};
use portgraph::{multiportgraph, LinkView, PortView};

use super::internal::{HugrInternals, HugrMutInternals};
use super::internal::HugrInternals;
use super::{
Hugr, HugrError, HugrMut, NodeMetadata, NodeMetadataMap, ValidationError, DEFAULT_OPTYPE,
};
Expand Down Expand Up @@ -509,7 +509,6 @@ pub trait ExtractHugr: HugrView + Sized {
let old_root = hugr.root();
let new_root = hugr.insert_from_view(old_root, &self).new_root;
hugr.set_root(new_root);
hugr.set_num_ports(new_root, 0, 0);
hugr.remove_node(old_root);
hugr
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ source: hugr-core/src/hugr/views/tests.rs
expression: h.dot_string()
---
digraph {
0 [shape=plain label=<<table border="1"><tr><td align="text" border="0" colspan="1">(0) CFG</td></tr></table>>]
0 [shape=plain label=<<table border="1"><tr><td port="in0" align="text" colspan="1" cellpadding="1" >0: usize</td></tr><tr><td align="text" border="0" colspan="1">(0) CFG</td></tr><tr><td port="out0" align="text" colspan="1" cellpadding="1" >0: usize</td></tr></table>>]
1 [shape=plain label=<<table border="1"><tr><td port="in0" align="text" colspan="1" cellpadding="1" border="0">0</td></tr><tr><td align="text" border="0" colspan="1">(1) ExitBlock</td></tr></table>>]
2 [shape=plain label=<<table border="1"><tr><td port="in0" align="text" colspan="2" cellpadding="1" border="0">0</td></tr><tr><td align="text" border="0" colspan="2">(2) DataflowBlock</td></tr><tr><td port="out0" align="text" colspan="1" cellpadding="1" border="0">0</td><td port="out1" align="text" colspan="1" cellpadding="1" border="0">1</td></tr></table>>]
2:out0 -> 6:in0 [style="dashed"]
Expand Down Expand Up @@ -44,4 +44,3 @@ hier8 [shape=plain label="8"]
hier9 [shape=plain label="9"]
hier10 [shape=plain label="10"]
}

Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
---
source: hugr-core/views/tests.rs
source: hugr-core/src/hugr/views/tests.rs
expression: h.dot_string()
---
digraph {
0 [shape=plain label=<<table border="1"><tr><td align="text" border="0" colspan="1">(0) DFG</td></tr></table>>]
0 [shape=plain label=<<table border="1"><tr><td port="in0" align="text" colspan="2" cellpadding="1" >0: qubit</td><td port="in1" align="text" colspan="2" cellpadding="1" >1: qubit</td></tr><tr><td align="text" border="0" colspan="4">(0) DFG</td></tr><tr><td port="out0" align="text" colspan="2" cellpadding="1" >0: qubit</td><td port="out1" align="text" colspan="2" cellpadding="1" >1: qubit</td></tr></table>>]
1 [shape=plain label=<<table border="1"><tr><td align="text" border="0" colspan="2">(1) Input</td></tr><tr><td port="out0" align="text" colspan="1" cellpadding="1" >0: qubit</td><td port="out1" align="text" colspan="1" cellpadding="1" >1: qubit</td></tr></table>>]
1:out0 -> 3:in0 [style=""]
1:out1 -> 3:in1 [style=""]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ source: hugr-core/src/hugr/views/tests.rs
expression: h.dot_string()
---
digraph {
0 [shape=plain label=<<table border="1"><tr><td align="text" border="0" colspan="1">(0) DFG</td></tr></table>>]
0 [shape=plain label=<<table border="1"><tr><td port="in0" align="text" colspan="1" cellpadding="1" >0: []+[]</td></tr><tr><td align="text" border="0" colspan="1">(0) DFG</td></tr><tr><td port="out0" align="text" colspan="1" cellpadding="1" >0: []+[]</td></tr></table>>]
1 [shape=plain label=<<table border="1"><tr><td align="text" border="0" colspan="1">(1) Input</td></tr><tr><td port="out0" align="text" colspan="1" cellpadding="1" >0: []+[]</td></tr></table>>]
1:out0 -> 2:in0 [style=""]
2 [shape=plain label=<<table border="1"><tr><td port="in0" align="text" colspan="1" cellpadding="1" >0: []+[]</td></tr><tr><td align="text" border="0" colspan="1">(2) Output</td></tr></table>>]
Expand Down

0 comments on commit a66c4b9

Please sign in to comment.