Fix QuantumCircuit.depth with zero-operands and Expr nodes (#12429)

This causes `QuantumCircuit.depth` to correctly handle cases where a
circuit instruction has zero operands (such as `GlobalPhaseGate`), and
to treat classical bits and real-time variables used inside `Expr`
conditions as part of the depth calculations.  This is in line with
`DAGCircuit`.

This commit still does not add the same `recurse` argument from
`DAGCircuit.depth`, because the arguments for not adding it to
`QuantumCircuit.depth` at the time still hold; there is no clear meaning
to it for general control flow from a user's perspective, and it was
only added to the `DAGCircuit` methods because there it is more of a
proxy for optimising over all possible inner blocks.

qiskit/circuit/quantumcircuit.py

@@ -45,7 +45,7 @@
 from qiskit.circuit.exceptions import CircuitError
 from . import _classical_resource_map
 from ._utils import sort_parameters
-from .controlflow import ControlFlowOp
+from .controlflow import ControlFlowOp, _builder_utils
 from .controlflow.builder import CircuitScopeInterface, ControlFlowBuilderBlock
 from .controlflow.break_loop import BreakLoopOp, BreakLoopPlaceholder
 from .controlflow.continue_loop import ContinueLoopOp, ContinueLoopPlaceholder
@@ -3307,6 +3307,9 @@ def depth(
     ) -> int:
         """Return circuit depth (i.e., length of critical path).
 
+        .. warning::
+            This operation is not well defined if the circuit contains control-flow operations.
+
         Args:
             filter_function: A function to decide which instructions count to increase depth.
                 Should take as a single positional input a :class:`CircuitInstruction`.
@@ -3332,59 +3335,40 @@ def depth(
 
                 assert qc.depth(lambda instr: len(instr.qubits) > 1) == 1
         """
-        # Assign each bit in the circuit a unique integer
-        # to index into op_stack.
-        bit_indices: dict[Qubit | Clbit, int] = {
-            bit: idx for idx, bit in enumerate(self.qubits + self.clbits)
+        obj_depths = {
+            obj: 0 for objects in (self.qubits, self.clbits, self.iter_vars()) for obj in objects
         }
 
-        # If no bits, return 0
-        if not bit_indices:
-            return 0
+        def update_from_expr(objects, node):
+            for var in expr.iter_vars(node):
+                if var.standalone:
+                    objects.add(var)
+                else:
+                    objects.update(_builder_utils.node_resources(var).clbits)
 
-        # A list that holds the height of each qubit
-        # and classical bit.
-        op_stack = [0] * len(bit_indices)
-
-        # Here we are playing a modified version of
-        # Tetris where we stack gates, but multi-qubit
-        # gates, or measurements have a block for each
-        # qubit or cbit that are connected by a virtual
-        # line so that they all stacked at the same depth.
-        # Conditional gates act on all cbits in the register
-        # they are conditioned on.
-        # The max stack height is the circuit depth.
         for instruction in self._data:
-            levels = []
-            reg_ints = []
-            for ind, reg in enumerate(instruction.qubits + instruction.clbits):
-                # Add to the stacks of the qubits and
-                # cbits used in the gate.
-                reg_ints.append(bit_indices[reg])
-                if filter_function(instruction):
-                    levels.append(op_stack[reg_ints[ind]] + 1)
-                else:
-                    levels.append(op_stack[reg_ints[ind]])
-            # Assuming here that there is no conditional
-            # snapshots or barriers ever.
-            if getattr(instruction.operation, "condition", None):
-                # Controls operate over all bits of a classical register
-                # or over a single bit
-                if isinstance(instruction.operation.condition[0], Clbit):
-                    condition_bits = [instruction.operation.condition[0]]
+            objects = set(itertools.chain(instruction.qubits, instruction.clbits))
+            if (condition := getattr(instruction.operation, "condition", None)) is not None:
+                objects.update(_builder_utils.condition_resources(condition).clbits)
+                if isinstance(condition, expr.Expr):
+                    update_from_expr(objects, condition)
                 else:
-                    condition_bits = instruction.operation.condition[0]
-                for cbit in condition_bits:
-                    idx = bit_indices[cbit]
-                    if idx not in reg_ints:
-                        reg_ints.append(idx)
-                        levels.append(op_stack[idx] + 1)
-
-            max_level = max(levels)
-            for ind in reg_ints:
-                op_stack[ind] = max_level
-
-        return max(op_stack)
+                    objects.update(_builder_utils.condition_resources(condition).clbits)
+            elif isinstance(instruction.operation, SwitchCaseOp):
+                update_from_expr(objects, expr.lift(instruction.operation.target))
+            elif isinstance(instruction.operation, Store):
+                update_from_expr(objects, instruction.operation.lvalue)
+                update_from_expr(objects, instruction.operation.rvalue)
+
+            # If we're counting this as adding to depth, do so.  If not, it still functions as a
+            # data synchronisation point between the objects (think "barrier"), so the depths still
+            # get updated to match the current max over the affected objects.
+            new_depth = max((obj_depths[obj] for obj in objects), default=0)
+            if filter_function(instruction):
+                new_depth += 1
+            for obj in objects:
+                obj_depths[obj] = new_depth
+        return max(obj_depths.values(), default=0)
 
     def width(self) -> int:
         """Return number of qubits plus clbits in circuit.

releasenotes/notes/fix-qc-depth-0q-cdcc9aa14e237e68.yaml

@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    :meth:`.QuantumCircuit.depth` will now correctly handle operations that
+    do not have operands, such as :class:`.GlobalPhaseGate`.
+  - |
+    :meth:`.QuantumCircuit.depth` will now count the variables and clbits
+    used in real-time expressions as part of the depth calculation.

test/python/circuit/test_circuit_properties.py

@@ -17,7 +17,8 @@
 
 from qiskit import QuantumRegister, ClassicalRegister, QuantumCircuit, pulse
 from qiskit.circuit import Clbit
-from qiskit.circuit.library import RXGate, RYGate
+from qiskit.circuit.classical import expr, types
+from qiskit.circuit.library import RXGate, RYGate, GlobalPhaseGate
 from qiskit.circuit.exceptions import CircuitError
 from test import QiskitTestCase  # pylint: disable=wrong-import-order
 
@@ -638,6 +639,58 @@ def test_circuit_depth_first_qubit(self):
         circ.measure(1, 0)
         self.assertEqual(circ.depth(lambda x: circ.qubits[0] in x.qubits), 3)
 
+    def test_circuit_depth_0_operands(self):
+        """Test that the depth can be found even with zero-bit operands."""
+        qc = QuantumCircuit(2, 2)
+        qc.append(GlobalPhaseGate(0.0), [], [])
+        qc.append(GlobalPhaseGate(0.0), [], [])
+        qc.append(GlobalPhaseGate(0.0), [], [])
+        self.assertEqual(qc.depth(), 0)
+        qc.measure([0, 1], [0, 1])
+        self.assertEqual(qc.depth(), 1)
+
+    def test_circuit_depth_expr_condition(self):
+        """Test that circuit depth respects `Expr` conditions in `IfElseOp`."""
+        # Note that the "depth" of control-flow operations is not well defined, so the assertions
+        # here are quite weak.  We're mostly aiming to match legacy behaviour of `c_if` for cases
+        # where there's a single instruction within the conditional.
+        qc = QuantumCircuit(2, 2)
+        a = qc.add_input("a", types.Bool())
+        with qc.if_test(a):
+            qc.x(0)
+        with qc.if_test(expr.logic_and(a, qc.clbits[0])):
+            qc.x(1)
+        self.assertEqual(qc.depth(), 2)
+        qc.measure([0, 1], [0, 1])
+        self.assertEqual(qc.depth(), 3)
+
+    def test_circuit_depth_expr_store(self):
+        """Test that circuit depth respects `Store`."""
+        qc = QuantumCircuit(3, 3)
+        a = qc.add_input("a", types.Bool())
+        qc.h(0)
+        qc.cx(0, 1)
+        qc.measure([0, 1], [0, 1])
+        # Note that `Store` is a "directive", so doesn't increase the depth by default, but does
+        # cause qubits 0,1; clbits 0,1 and 'a' to all be depth 3 at this point.
+        qc.store(a, qc.clbits[0])
+        qc.store(a, expr.logic_and(a, qc.clbits[1]))
+        # ... so this use of 'a' should make it depth 4.
+        with qc.if_test(a):
+            qc.x(2)
+        self.assertEqual(qc.depth(), 4)
+
+    def test_circuit_depth_switch(self):
+        """Test that circuit depth respects the `target` of `SwitchCaseOp`."""
+        qc = QuantumCircuit(QuantumRegister(3, "q"), ClassicalRegister(3, "c"))
+        a = qc.add_input("a", types.Uint(3))
+
+        with qc.switch(expr.bit_and(a, qc.cregs[0])) as case:
+            with case(case.DEFAULT):
+                qc.x(0)
+        qc.measure(1, 0)
+        self.assertEqual(qc.depth(), 2)
+
     def test_circuit_size_empty(self):
         """Circuit.size should return 0 for an empty circuit."""
         size = 4