Cache dag qubits/clbits (#4535)

* cache dag.qubits and dag.clbits

* releasenotes

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Kevin Krsulich <kevin.krsulich@ibm.com>

qiskit/dagcircuit/dagcircuit.py

@@ -91,12 +91,22 @@ def __init__(self):
         # Edges carry wire labels (reg,idx) and each operation has
         # corresponding in- and out-edges with the same wire labels.
 
-        # Map of qreg name to QuantumRegister object
+        # Map of qreg/creg name to Register object.
         self.qregs = OrderedDict()
-
-        # Map of creg name to ClassicalRegister object
         self.cregs = OrderedDict()
 
+        # List of Qubit/Clbit wires that the DAG acts on.
+        class DummyCallableList(list):
+            """Dummy class so we can deprecate dag.qubits() and do
+            dag.qubits as property.
+            """
+            def __call__(self):
+                warnings.warn('dag.qubits() and dag.clbits() are no longer methods. Use '
+                              'dag.qubits and dag.clbits properties instead.', DeprecationWarning)
+                return self
+        self._qubits = DummyCallableList()  # TODO: make these a regular empty list [] after the
+        self._clbits = DummyCallableList()  # DeprecationWarning period, and remove name underscore.
+
         self._id_to_node = {}
 
         self._multi_graph = None
@@ -177,13 +187,17 @@ def from_networkx(cls, graph):
                                          node.cargs, node.condition)
         return dag
 
+    @property
     def qubits(self):
         """Return a list of qubits (as a list of Qubit instances)."""
-        return [qubit for qreg in self.qregs.values() for qubit in qreg]
+        # TODO: remove this property after DeprecationWarning period (~9/2020)
+        return self._qubits
 
+    @property
     def clbits(self):
         """Return a list of classical bits (as a list of Clbit instances)."""
-        return [clbit for creg in self.cregs.values() for clbit in creg]
+        # TODO: remove this property after DeprecationWarning period (~9/2020)
+        return self._clbits
 
     @property
     def wires(self):
@@ -212,6 +226,7 @@ def add_qreg(self, qreg):
             raise DAGCircuitError("duplicate register %s" % qreg.name)
         self.qregs[qreg.name] = qreg
         for j in range(qreg.size):
+            self.qubits.append(qreg[j])
             self._add_wire(qreg[j])
 
     def add_creg(self, creg):
@@ -222,6 +237,7 @@ def add_creg(self, creg):
             raise DAGCircuitError("duplicate register %s" % creg.name)
         self.cregs[creg.name] = creg
         for j in range(creg.size):
+            self.clbits.append(creg[j])
             self._add_wire(creg[j])
 
     def _add_wire(self, wire):
@@ -546,8 +562,8 @@ def compose(self, other, edge_map=None, qubits=None, clbits=None, front=False, i
         if front:
             raise DAGCircuitError("Front composition not supported yet.")
 
-        if len(other.qubits()) > len(self.qubits()) or \
-           len(other.clbits()) > len(self.clbits()):
+        if len(other.qubits) > len(self.qubits) or \
+           len(other.clbits) > len(self.clbits):
             raise DAGCircuitError("Trying to compose with another DAGCircuit "
                                   "which has more 'in' edges.")
 
@@ -560,15 +576,15 @@ def compose(self, other, edge_map=None, qubits=None, clbits=None, front=False, i
             qubits = []
         if clbits is None:
             clbits = []
-        qubit_map = {other.qubits()[i]: (self.qubits()[q] if isinstance(q, int) else q)
+        qubit_map = {other.qubits[i]: (self.qubits[q] if isinstance(q, int) else q)
                      for i, q in enumerate(qubits)}
-        clbit_map = {other.clbits()[i]: (self.clbits()[c] if isinstance(c, int) else c)
+        clbit_map = {other.clbits[i]: (self.clbits[c] if isinstance(c, int) else c)
                      for i, c in enumerate(clbits)}
         edge_map = edge_map or {**qubit_map, **clbit_map} or None
         # if no edge_map, try to do a 1-1 mapping in order
         if edge_map is None:
-            identity_qubit_map = dict(zip(other.qubits(), self.qubits()))
-            identity_clbit_map = dict(zip(other.clbits(), self.clbits()))
+            identity_qubit_map = dict(zip(other.qubits, self.qubits))
+            identity_clbit_map = dict(zip(other.clbits, self.clbits))
             edge_map = {**identity_qubit_map, **identity_clbit_map}
 
         # Check the edge_map for duplicate values

qiskit/dagcircuit/dagdependency.py

@@ -75,12 +75,14 @@ def __init__(self):
         # represent non-commutativity between two gates.
         self._multi_graph = rx.PyDAG()
 
-        # Map of qreg name to QuantumRegister object
+        # Map of qreg/creg name to Register object.
         self.qregs = OrderedDict()
-
-        # Map of creg name to ClassicalRegister object
         self.cregs = OrderedDict()
 
+        # List of all Qubit/Clbit wires.
+        self.qubits = []
+        self.clbits = []
+
     def to_networkx(self):
         """Returns a copy of the DAGDependency in networkx format."""
         # For backwards compatibility, return networkx structure from terra 0.12
@@ -100,14 +102,6 @@ def to_retworkx(self):
         """ Returns the DAGDependency in retworkx format."""
         return self._multi_graph
 
-    def qubits(self):
-        """Return a list of qubits (as a list of Qubit instances)."""
-        return [qubit for qreg in self.qregs.values() for qubit in qreg]
-
-    def clbits(self):
-        """Return a list of classical bits (as a list of Clbit instances)."""
-        return [clbit for creg in self.cregs.values() for clbit in creg]
-
     def size(self):
         """ Returns the number of gates in the circuit"""
         return len(self._multi_graph)
@@ -127,14 +121,18 @@ def add_qreg(self, qreg):
         if qreg.name in self.qregs:
             raise DAGDependencyError("duplicate register %s" % qreg.name)
         self.qregs[qreg.name] = qreg
+        for j in range(qreg.size):
+            self.qubits.append(qreg[j])
 
     def add_creg(self, creg):
-        """Add all wires in a classical register."""
+        """Add clbits in a classical register."""
         if not isinstance(creg, ClassicalRegister):
             raise DAGDependencyError("not a ClassicalRegister instance.")
         if creg.name in self.cregs:
             raise DAGDependencyError("duplicate register %s" % creg.name)
         self.cregs[creg.name] = creg
+        for j in range(creg.size):
+            self.clbits.append(creg[j])
 
     def _add_multi_graph_node(self, node):
         """
@@ -298,19 +296,16 @@ def add_op_node(self, operation, qargs, cargs):
             qargs (list[Qubit]): list of qubits on which the operation acts
             cargs (list[Clbit]): list of classical wires to attach to.
         """
-        all_qubits = self.qubits()
-        all_clbits = self.clbits()
-
         directives = ['measure', 'barrier', 'snapshot']
         if operation.name not in directives:
             qindices_list = []
             for elem in qargs:
-                qindices_list.append(all_qubits.index(elem))
+                qindices_list.append(self.qubits.index(elem))
             if operation.condition:
-                for clbit in all_clbits:
+                for clbit in self.clbits:
                     if clbit.register == operation.condition[0]:
-                        initial = all_clbits.index(clbit)
-                        final = all_clbits.index(clbit) + clbit.register.size
+                        initial = self.clbits.index(clbit)
+                        final = self.clbits.index(clbit) + clbit.register.size
                         cindices_list = range(initial, final)
                         break
             else:

qiskit/transpiler/passes/layout/csp_layout.py

@@ -102,7 +102,7 @@ def __init__(self, coupling_map, strict_direction=False, seed=None, call_limit=1
         self.seed = seed
 
     def run(self, dag):
-        qubits = dag.qubits()
+        qubits = dag.qubits
         cxs = set()
 
         for gate in dag.two_qubit_ops():

qiskit/transpiler/passes/layout/noise_adaptive_layout.py

@@ -140,7 +140,7 @@ def _create_program_graph(self, dag):
         number of CNOTs between the pair.
         """
         idx = 0
-        for q in dag.qubits():
+        for q in dag.qubits:
             self.qarg_to_id[q.register.name + str(q.index)] = idx
             idx += 1
         for gate in dag.two_qubit_ops():
@@ -252,7 +252,7 @@ def run(self, dag):
                 self.prog2hw[qid] = self.available_hw_qubits[0]
                 self.available_hw_qubits.remove(self.prog2hw[qid])
         layout = Layout()
-        for q in dag.qubits():
+        for q in dag.qubits:
             pid = self._qarg_to_id(q)
             hwid = self.prog2hw[pid]
             layout[q] = hwid

qiskit/transpiler/passes/optimization/consolidate_blocks.py

@@ -63,7 +63,7 @@ def run(self, dag):
             new_dag.add_creg(creg)
 
         # compute ordered indices for the global circuit wires
-        global_index_map = {wire: idx for idx, wire in enumerate(dag.qubits())}
+        global_index_map = {wire: idx for idx, wire in enumerate(dag.qubits)}
 
         blocks = self.property_set['block_list']
         # just to make checking if a node is in any block easier

qiskit/transpiler/passes/optimization/hoare_opt.py

@@ -69,7 +69,7 @@ def _initialize(self, dag):
         Args:
             dag (DAGCircuit): input DAG to get qubits from
         """
-        for qbt in dag.qubits():
+        for qbt in dag.qubits:
             self.gatenum[qbt.index] = 0
             self.variables[qbt.index] = []
             self.gatecache[qbt.index] = []
@@ -338,6 +338,6 @@ def run(self, dag):
         self._initialize(dag)
         self._traverse_dag(dag)
         if self.size > 1:
-            for qbt in dag.qubits():
+            for qbt in dag.qubits:
                 self._multigate_opt(dag, qbt.index)
         return dag

qiskit/transpiler/passes/routing/basic_swap.py

@@ -60,7 +60,7 @@ def run(self, dag):
         if len(dag.qregs) != 1 or dag.qregs.get('q', None) is None:
             raise TranspilerError('Basic swap runs on physical circuits only')
 
-        if len(dag.qubits()) > len(self.coupling_map.physical_qubits):
+        if len(dag.qubits) > len(self.coupling_map.physical_qubits):
             raise TranspilerError('The layout does not match the amount of qubits in the DAG')
 
         canonical_register = dag.qregs['q']
@@ -92,14 +92,14 @@ def run(self, dag):
                                                         cargs=[])
 
                     # layer insertion
-                    order = current_layout.reorder_bits(new_dag.qubits())
+                    order = current_layout.reorder_bits(new_dag.qubits)
                     new_dag.compose(swap_layer, qubits=order)
 
                     # update current_layout
                     for swap in range(len(path) - 2):
                         current_layout.swap(path[swap], path[swap + 1])
 
-            order = current_layout.reorder_bits(new_dag.qubits())
+            order = current_layout.reorder_bits(new_dag.qubits)
             new_dag.compose(subdag, qubits=order)
 
         return new_dag

qiskit/transpiler/passes/routing/layout_transformation.py

@@ -79,7 +79,7 @@ def run(self, dag):
         if len(dag.qregs) != 1 or dag.qregs.get('q', None) is None:
             raise TranspilerError('LayoutTransform runs on physical circuits only')
 
-        if len(dag.qubits()) > len(self.coupling_map.physical_qubits):
+        if len(dag.qubits) > len(self.coupling_map.physical_qubits):
             raise TranspilerError('The layout does not match the amount of qubits in the DAG')
 
         from_layout = self.from_layout
@@ -102,7 +102,7 @@ def run(self, dag):
 
         perm_circ = self.token_swapper.permutation_circuit(permutation, self.trials)
 
-        edge_map = {vqubit: dag.qubits()[pqubit]
+        edge_map = {vqubit: dag.qubits[pqubit]
                     for (pqubit, vqubit) in perm_circ.inputmap.items()}
         dag.compose(perm_circ.circuit, edge_map=edge_map)
         return dag

qiskit/transpiler/passes/routing/lookahead_swap.py

@@ -93,7 +93,7 @@ def run(self, dag):
         if len(dag.qregs) != 1 or dag.qregs.get('q', None) is None:
             raise TranspilerError('Lookahead swap runs on physical circuits only')
 
-        if len(dag.qubits()) > len(self.coupling_map.physical_qubits):
+        if len(dag.qubits) > len(self.coupling_map.physical_qubits):
             raise TranspilerError('The layout does not match the amount of qubits in the DAG')
 
         canonical_register = dag.qregs['q']

qiskit/transpiler/passes/routing/stochastic_swap.py

@@ -87,7 +87,7 @@ def run(self, dag):
         if len(dag.qregs) != 1 or dag.qregs.get('q', None) is None:
             raise TranspilerError('StochasticSwap runs on physical circuits only')
 
-        if len(dag.qubits()) > len(self.coupling_map.physical_qubits):
+        if len(dag.qubits) > len(self.coupling_map.physical_qubits):
             raise TranspilerError('The layout does not match the amount of qubits in the DAG')
 
         canonical_register = dag.qregs['q']
@@ -265,7 +265,7 @@ def _layer_update(self, i, best_layout, best_depth,
         for creg in layer_circuit.cregs.values():
             dagcircuit_output.add_creg(creg)
 
-        order = layout.reorder_bits(dagcircuit_output.qubits())
+        order = layout.reorder_bits(dagcircuit_output.qubits)
         dagcircuit_output.compose(layer_circuit, qubits=order)
 
         return dagcircuit_output

qiskit/transpiler/passes/utils/barrier_before_final_measurements.py

@@ -59,7 +59,7 @@ def run(self, dag):
 
         # Add a barrier across all qubits so swap mapper doesn't add a swap
         # from an unmeasured qubit after a measure.
-        final_qubits = dag.qubits()
+        final_qubits = dag.qubits
 
         barrier_layer.apply_operation_back(
             Barrier(len(final_qubits)), list(final_qubits), [])

qiskit/visualization/utils.py

@@ -125,8 +125,8 @@ def _get_layered_instructions(circuit, reverse_bits=False,
 
     dag = circuit_to_dag(circuit)
     ops = []
-    qregs = dag.qubits()
-    cregs = dag.clbits()
+    qregs = dag.qubits
+    cregs = dag.clbits
 
     if justify == 'none':
         for node in dag.topological_op_nodes():
@@ -198,7 +198,7 @@ def __init__(self, dag, justification):
         """Create spool"""
         super(_LayerSpooler, self).__init__()
         self.dag = dag
-        self.qregs = dag.qubits()
+        self.qregs = dag.qubits
         self.justification = justification
 
         if self.justification == 'left':

releasenotes/notes/deprecate-dag-qubits-method-0a61b37fa4a5bcd5.yaml

@@ -0,0 +1,7 @@
+---
+deprecations:
+  - |
+   :meth:`~qiskit.dagcircuit.DAGCircuit.qubits` and
+   :meth:`~qiskit.dagcircuit.DAGCircuit.clbits` have been deprecated
+   as methods. They are now properties of the dag, and are cached so
+   accessing them is much faster.

test/python/dagcircuit/test_dagdependency.py

@@ -71,12 +71,12 @@ def test_dag_get_qubits(self):
         dag.add_qreg(QuantumRegister(1, 'qr3'))
         dag.add_qreg(QuantumRegister(1, 'qr4'))
         dag.add_qreg(QuantumRegister(1, 'qr6'))
-        self.assertListEqual(dag.qubits(), [QuantumRegister(1, 'qr1')[0],
-                                            QuantumRegister(1, 'qr10')[0],
-                                            QuantumRegister(1, 'qr0')[0],
-                                            QuantumRegister(1, 'qr3')[0],
-                                            QuantumRegister(1, 'qr4')[0],
-                                            QuantumRegister(1, 'qr6')[0]])
+        self.assertListEqual(dag.qubits, [QuantumRegister(1, 'qr1')[0],
+                                          QuantumRegister(1, 'qr10')[0],
+                                          QuantumRegister(1, 'qr0')[0],
+                                          QuantumRegister(1, 'qr3')[0],
+                                          QuantumRegister(1, 'qr4')[0],
+                                          QuantumRegister(1, 'qr6')[0]])
 
     def test_add_reg_duplicate(self):
         """add_qreg with the same register twice is not allowed."""