Skip to content

Commit 8e6200a

Browse files
authored
Simplify return type of DAGCircuit::control_flow_op_nodes (Qiskit#13892)
Technically this is a breaking change, not because of the type change (we're returning a subset of what we previously did), but because the docstring previously _explicitly_ said that `None` was used for the empty list. In practice, this is a bit of a footgun (as I found), and inconsistent with the other `DAGCircuit` methods. It's not clear that it has a performance advantage; technically it can save a single Python empty-list allocation (`None` is a singleton), but this cost is pretty trivial.
1 parent ca1297e commit 8e6200a

File tree

4 files changed

+35
-35
lines changed

4 files changed

+35
-35
lines changed

crates/circuit/src/dag_circuit.rs

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3628,28 +3628,24 @@ def _format(operand):
36283628
/// Get a list of "op" nodes in the dag that contain control flow instructions.
36293629
///
36303630
/// Returns:
3631-
/// list[DAGOpNode] | None: The list of dag nodes containing control flow ops. If there
3632-
/// are no control flow nodes None is returned
3633-
fn control_flow_op_nodes(&self, py: Python) -> PyResult<Option<Vec<Py<PyAny>>>> {
3634-
if self.has_control_flow() {
3635-
let result: PyResult<Vec<Py<PyAny>>> = self
3636-
.dag
3637-
.node_references()
3638-
.filter_map(|(node_index, node_type)| match node_type {
3639-
NodeType::Operation(ref node) => {
3640-
if node.op.control_flow() {
3641-
Some(self.unpack_into(py, node_index, node_type))
3642-
} else {
3643-
None
3644-
}
3645-
}
3646-
_ => None,
3647-
})
3648-
.collect();
3649-
Ok(Some(result?))
3650-
} else {
3651-
Ok(None)
3631+
/// list[DAGOpNode]: The list of dag nodes containing control flow ops.
3632+
fn control_flow_op_nodes(&self, py: Python) -> PyResult<Vec<Py<PyAny>>> {
3633+
if !self.has_control_flow() {
3634+
return Ok(vec![]);
36523635
}
3636+
self.dag
3637+
.node_references()
3638+
.filter_map(|(node_index, node_type)| match node_type {
3639+
NodeType::Operation(ref node) => {
3640+
if node.op.control_flow() {
3641+
Some(self.unpack_into(py, node_index, node_type))
3642+
} else {
3643+
None
3644+
}
3645+
}
3646+
_ => None,
3647+
})
3648+
.collect()
36533649
}
36543650

36553651
/// Get the list of gate nodes in the dag.

qiskit/transpiler/passes/optimization/consolidate_blocks.py

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -138,11 +138,9 @@ def _handle_control_flow_ops(self, dag):
138138
pass_manager.append(Collect2qBlocks())
139139

140140
pass_manager.append(self)
141-
control_flow_nodes = dag.control_flow_op_nodes()
142-
if control_flow_nodes is not None:
143-
for node in control_flow_nodes:
144-
dag.substitute_node(
145-
node,
146-
node.op.replace_blocks(pass_manager.run(block) for block in node.op.blocks),
147-
)
141+
for node in dag.control_flow_op_nodes():
142+
dag.substitute_node(
143+
node,
144+
node.op.replace_blocks(pass_manager.run(block) for block in node.op.blocks),
145+
)
148146
return dag

qiskit/transpiler/passes/utils/control_flow.py

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,8 @@ def out(self, dag):
5454
def bound_wrapped_method(dag):
5555
return out(self, dag)
5656

57-
control_flow_nodes = dag.control_flow_op_nodes()
58-
if control_flow_nodes is not None:
59-
for node in control_flow_nodes:
60-
dag.substitute_node(
61-
node,
62-
map_blocks(bound_wrapped_method, node.op),
63-
)
57+
for node in dag.control_flow_op_nodes():
58+
dag.substitute_node(node, map_blocks(bound_wrapped_method, node.op))
6459
return method(self, dag)
6560

6661
return out
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
upgrade_circuits:
3+
- |
4+
:meth:`.DAGCircuit.control_flow_op_nodes` now always returns a list, even if empty. Previously,
5+
it returned ``None`` if empty, and never returned the empty list, which required special handling.
6+
If you need to explicitly test for emptiness in both Qiskit 1.x and 2.x, you can do::
7+
8+
control_flow_nodes = dag.control_flow_op_nodes()
9+
if not control_flow_nodes:
10+
# There are no control-flow nodes.
11+
pass

0 commit comments

Comments
 (0)