Skip to content

Commit c8c88cd

Browse files
authored
fix: Inconsistent behaviour in SiblingSubgraph::from_nodes (#2011)
`SiblingSubgraph::from_nodes` and `from_node` have different definitions. The former takes a set of nodes and defines a subgraph where the inputs and outputs correspond to all edges connecting the chosen nodes to external ones. When a node has a disconnected port, it is ignored. The latter does not check edges. It takes a single node and creates a subgraph using the dataflow signature of the operation. Both definitions are valid and useful on their own, when calling `from_nodes` with a single-element vector we redirected the call to `from_node`, resulting in different port definitions. This PR fixes that.
1 parent c5423ed commit c8c88cd

File tree

1 file changed

+37
-7
lines changed

1 file changed

+37
-7
lines changed

hugr-core/src/hugr/views/sibling_subgraph.rs

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ impl<N: HugrNode> SiblingSubgraph<N> {
189189
.collect_vec();
190190
validate_subgraph(hugr, &nodes, &inputs, &outputs)?;
191191

192-
if !subpg.is_convex_with_checker(checker) {
192+
if nodes.len() > 1 && !subpg.is_convex_with_checker(checker) {
193193
return Err(InvalidSubgraph::NotConvex);
194194
}
195195

@@ -238,12 +238,9 @@ impl<N: HugrNode> SiblingSubgraph<N> {
238238
) -> Result<Self, InvalidSubgraph<N>> {
239239
let nodes = nodes.into();
240240

241-
// If there's one or less nodes, we don't need to check convexity.
242-
match nodes.as_slice() {
243-
[] => return Err(InvalidSubgraph::EmptySubgraph),
244-
[node] => return Ok(Self::from_node(*node, hugr)),
245-
_ => {}
246-
};
241+
if nodes.is_empty() {
242+
return Err(InvalidSubgraph::EmptySubgraph);
243+
}
247244

248245
let nodes_set = nodes.iter().copied().collect::<HashSet<_>>();
249246
let incoming_edges = nodes
@@ -1239,4 +1236,37 @@ mod tests {
12391236
let rep = subg.create_simple_replacement(&h, replacement).unwrap();
12401237
rep.apply(&mut h).unwrap();
12411238
}
1239+
1240+
/// Test the behaviour of the sibling subgraph when built from a single node.
1241+
#[test]
1242+
fn single_node_subgraph() {
1243+
// A hugr with a single NOT operation, with disconnected output.
1244+
let mut b = DFGBuilder::new(
1245+
Signature::new(bool_t(), type_row![])
1246+
.with_extension_delta(crate::std_extensions::logic::EXTENSION_ID),
1247+
)
1248+
.unwrap();
1249+
let inw = b.input_wires().exactly_one().unwrap();
1250+
let not_n = b.add_dataflow_op(LogicOp::Not, [inw]).unwrap();
1251+
// Unconnected output, discarded
1252+
let h = b.finish_hugr_with_outputs([]).unwrap();
1253+
1254+
// When built with `from_node`, the subgraph's signature is the same as the node's.
1255+
// (bool input, bool output)
1256+
let subg = SiblingSubgraph::from_node(not_n.node(), &h);
1257+
assert_eq!(subg.nodes().len(), 1);
1258+
assert_eq!(
1259+
subg.signature(&h).io(),
1260+
Signature::new(vec![bool_t()], vec![bool_t()]).io()
1261+
);
1262+
1263+
// `from_nodes` is different, is it only uses incoming and outgoing edges to compute the signature.
1264+
// In this case, the output is disconnected, so it is not part of the subgraph signature.
1265+
let subg = SiblingSubgraph::try_from_nodes([not_n.node()], &h).unwrap();
1266+
assert_eq!(subg.nodes().len(), 1);
1267+
assert_eq!(
1268+
subg.signature(&h).io(),
1269+
Signature::new(vec![bool_t()], vec![]).io()
1270+
);
1271+
}
12421272
}

0 commit comments

Comments
 (0)