From 1567e4e7e72d51688305f6adac7738f9c3579960 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Fri, 19 Nov 2021 18:08:37 -0500 Subject: [PATCH] Make internal Layout and CouplingMap attrs slotted and adjust passes for fast access (#7036) * Make internal layout and coupling map attributes public and slotted This commit updates the layout and coupling map classes to tune them for performance when used by transpiler passes. Right now the user facing api is doing a bunch of extra validation to provide helpful user messages and the internal data structures of both classes are hidden behind this api. This can add a bunch of overhead to transpiler passes (specifically routing passes) that are using these APIs in a loop. To address this commit changes these internal properties to public attributes and adds slots to the classes to speed up direct attribute access. This will enable us to tune transpiler passes to access these attributes more easily and faster. The follow on step here is to update any transpiler passes using the user facing api to access the attributes directly. Fixes #7035 * Update pass access patterns for performance * Move connectivity check inside distance matrix rebuild condition * Adjust array access pattern to avoid temporary object contruction * Make attributes private again * Update sabre layout virtual bit usage * Apply suggestions from code review Co-authored-by: Kevin Krsulich * Get mapping from layout once outside of loop This commit updates the layout usage in ApplyLayout and Layout2QDistance to access the internal map once and store that in a local variable which is then accessed from the loop using the layout. Co-authored-by: Kevin Krsulich * Update qiskit/transpiler/passes/layout/sabre_layout.py Co-authored-by: Jake Lishman * Locally cache objects in compute_cost * Adjust access in full ancilla and layout2q passes to avoid private access * Adjust gate_direction to limit private access * Add comments about private access to Checkmap * Update qiskit/transpiler/passes/layout/layout_2q_distance.py Co-authored-by: Kevin Krsulich * Updates to sabre passes * Remove unecessary private access from stochastic swap * Add comment on private access to stochastic swap * Adjust distance matrix access Co-authored-by: Kevin Krsulich Co-authored-by: Jake Lishman --- qiskit/transpiler/coupling.py | 24 +++++++++++-------- qiskit/transpiler/layout.py | 4 +++- .../transpiler/passes/layout/apply_layout.py | 3 ++- .../passes/layout/full_ancilla_allocation.py | 12 +++++----- .../passes/layout/layout_2q_distance.py | 14 ++++++++--- .../transpiler/passes/layout/sabre_layout.py | 4 +--- .../transpiler/passes/routing/sabre_swap.py | 14 ++++++++--- .../passes/routing/stochastic_swap.py | 10 ++++---- qiskit/transpiler/passes/utils/check_map.py | 6 +++-- .../transpiler/passes/utils/gate_direction.py | 12 +++++++--- .../transpiler/preset_passmanagers/level2.py | 1 - .../transpiler/preset_passmanagers/level3.py | 1 - 12 files changed, 67 insertions(+), 38 deletions(-) diff --git a/qiskit/transpiler/coupling.py b/qiskit/transpiler/coupling.py index d6def6deb343..7324fe14abf1 100644 --- a/qiskit/transpiler/coupling.py +++ b/qiskit/transpiler/coupling.py @@ -39,6 +39,8 @@ class CouplingMap: to permitted CNOT gates """ + __slots__ = ("description", "graph", "_dist_matrix", "_qubit_list", "_size", "_is_symmetric") + def __init__(self, couplinglist=None, description=None): """ Create coupling graph. By default, the generated coupling has no nodes. @@ -59,7 +61,6 @@ def __init__(self, couplinglist=None, description=None): self._qubit_list = None # number of qubits in the graph self._size = None - # a sorted list of physical qubits (integers) in this coupling map self._is_symmetric = None if couplinglist is not None: @@ -159,19 +160,23 @@ def neighbors(self, physical_qubit): @property def distance_matrix(self): """Return the distance matrix for the coupling map.""" - if self._dist_matrix is None: - self._compute_distance_matrix() + self.compute_distance_matrix() return self._dist_matrix - def _compute_distance_matrix(self): + def compute_distance_matrix(self): """Compute the full distance matrix on pairs of nodes. The distance map self._dist_matrix is computed from the graph using - all_pairs_shortest_path_length. + all_pairs_shortest_path_length. This is normally handled internally + by the :attr:`~qiskit.transpiler.CouplingMap.distance_matrix` + attribute or the :meth:`~qiskit.transpiler.CouplingMap.distance` method + but can be called if you're accessing the distance matrix outside of + those or want to pre-generate it. """ - if not self.is_connected(): - raise CouplingError("coupling graph not connected") - self._dist_matrix = rx.digraph_distance_matrix(self.graph, as_undirected=True) + if self._dist_matrix is None: + if not self.is_connected(): + raise CouplingError("coupling graph not connected") + self._dist_matrix = rx.digraph_distance_matrix(self.graph, as_undirected=True) def distance(self, physical_qubit1, physical_qubit2): """Returns the undirected distance between physical_qubit1 and physical_qubit2. @@ -190,8 +195,7 @@ def distance(self, physical_qubit1, physical_qubit2): raise CouplingError("%s not in coupling graph" % physical_qubit1) if physical_qubit2 >= self.size(): raise CouplingError("%s not in coupling graph" % physical_qubit2) - if self._dist_matrix is None: - self._compute_distance_matrix() + self.compute_distance_matrix() return int(self._dist_matrix[physical_qubit1, physical_qubit2]) def shortest_undirected_path(self, physical_qubit1, physical_qubit2): diff --git a/qiskit/transpiler/layout.py b/qiskit/transpiler/layout.py index 93c95bc633de..139b566734d4 100644 --- a/qiskit/transpiler/layout.py +++ b/qiskit/transpiler/layout.py @@ -26,6 +26,8 @@ class Layout: """Two-ways dict to represent a Layout.""" + __slots__ = ("_regs", "_p2v", "_v2p") + def __init__(self, input_dict=None): """construct a Layout from a bijective dictionary, mapping virtual qubits to physical qubits""" @@ -239,7 +241,7 @@ def combine_into_edge_map(self, another_layout): """ edge_map = {} - for virtual, physical in self.get_virtual_bits().items(): + for virtual, physical in self._v2p.items(): if physical not in another_layout._p2v: raise LayoutError( "The wire_map_from_layouts() method does not support when the" diff --git a/qiskit/transpiler/passes/layout/apply_layout.py b/qiskit/transpiler/passes/layout/apply_layout.py index 63cf3b1fd710..f4f65af7338b 100644 --- a/qiskit/transpiler/passes/layout/apply_layout.py +++ b/qiskit/transpiler/passes/layout/apply_layout.py @@ -58,8 +58,9 @@ def run(self, dag): new_dag.add_clbits(dag.clbits) for creg in dag.cregs.values(): new_dag.add_creg(creg) + virtual_phsyical_map = layout.get_virtual_bits() for node in dag.topological_op_nodes(): - qargs = [q[layout[qarg]] for qarg in node.qargs] + qargs = [q[virtual_phsyical_map[qarg]] for qarg in node.qargs] new_dag.apply_operation_back(node.op, qargs, node.cargs) new_dag._global_phase = dag._global_phase diff --git a/qiskit/transpiler/passes/layout/full_ancilla_allocation.py b/qiskit/transpiler/passes/layout/full_ancilla_allocation.py index 50ff6be4773e..4c678af5ea78 100644 --- a/qiskit/transpiler/passes/layout/full_ancilla_allocation.py +++ b/qiskit/transpiler/passes/layout/full_ancilla_allocation.py @@ -65,19 +65,19 @@ def run(self, dag): if layout is None: raise TranspilerError('FullAncillaAllocation pass requires property_set["layout"].') + virtual_bits = layout.get_virtual_bits() + physical_bits = layout.get_physical_bits() if layout: - FullAncillaAllocation.validate_layout(layout.get_virtual_bits(), set(dag.qubits)) - layout_physical_qubits = list(range(max(layout.get_physical_bits()) + 1)) + FullAncillaAllocation.validate_layout(virtual_bits, set(dag.qubits)) + layout_physical_qubits = list(range(max(physical_bits) + 1)) else: layout_physical_qubits = [] - idle_physical_qubits = [ - q for q in layout_physical_qubits if q not in layout.get_physical_bits() - ] + idle_physical_qubits = [q for q in layout_physical_qubits if q not in physical_bits] if self.coupling_map: idle_physical_qubits = [ - q for q in self.coupling_map.physical_qubits if q not in layout.get_physical_bits() + q for q in self.coupling_map.physical_qubits if q not in physical_bits ] if idle_physical_qubits: diff --git a/qiskit/transpiler/passes/layout/layout_2q_distance.py b/qiskit/transpiler/passes/layout/layout_2q_distance.py index 4e6353a9d6af..f31b44591f0c 100644 --- a/qiskit/transpiler/passes/layout/layout_2q_distance.py +++ b/qiskit/transpiler/passes/layout/layout_2q_distance.py @@ -52,12 +52,20 @@ def run(self, dag): if layout is None: return + if self.coupling_map is None or len(self.coupling_map.graph) == 0: + self.property_set[self.property_name] = 0 + return + + self.coupling_map.compute_distance_matrix() + sum_distance = 0 + virtual_physical_map = layout.get_virtual_bits() + dist_matrix = self.coupling_map.distance_matrix for gate in dag.two_qubit_ops(): - physical_q0 = layout[gate.qargs[0]] - physical_q1 = layout[gate.qargs[1]] + physical_q0 = virtual_physical_map[gate.qargs[0]] + physical_q1 = virtual_physical_map[gate.qargs[1]] - sum_distance += self.coupling_map.distance(physical_q0, physical_q1) - 1 + sum_distance += dist_matrix[physical_q0, physical_q1] - 1 self.property_set[self.property_name] = sum_distance diff --git a/qiskit/transpiler/passes/layout/sabre_layout.py b/qiskit/transpiler/passes/layout/sabre_layout.py index 7c630987325d..9ba3fd1265c3 100644 --- a/qiskit/transpiler/passes/layout/sabre_layout.py +++ b/qiskit/transpiler/passes/layout/sabre_layout.py @@ -142,7 +142,5 @@ def _compose_layouts(self, initial_layout, pass_final_layout, qregs): """ trivial_layout = Layout.generate_trivial_layout(*qregs) qubit_map = Layout.combine_into_edge_map(initial_layout, trivial_layout) - final_layout = { - v: pass_final_layout[qubit_map[v]] for v, _ in initial_layout.get_virtual_bits().items() - } + final_layout = {v: pass_final_layout._v2p[qubit_map[v]] for v in initial_layout._v2p} return Layout(final_layout) diff --git a/qiskit/transpiler/passes/routing/sabre_swap.py b/qiskit/transpiler/passes/routing/sabre_swap.py index fd837184ef2d..6ab6e0ff282d 100644 --- a/qiskit/transpiler/passes/routing/sabre_swap.py +++ b/qiskit/transpiler/passes/routing/sabre_swap.py @@ -135,6 +135,7 @@ def __init__(self, coupling_map, heuristic="basic", seed=None, fake_run=False): self.applied_predecessors = None self.qubits_decay = None self._bit_indices = None + self.dist_matrix = None def run(self, dag): """Run the SabreSwap pass on `dag`. @@ -153,6 +154,8 @@ def run(self, dag): if len(dag.qubits) > self.coupling_map.size(): raise TranspilerError("More virtual qubits exist than physical.") + self.dist_matrix = self.coupling_map.distance_matrix + rng = np.random.default_rng(self.seed) # Preserve input DAG's name, regs, wire_map, etc. but replace the graph. @@ -183,7 +186,11 @@ def run(self, dag): for node in front_layer: if len(node.qargs) == 2: v0, v1 = node.qargs - if self.coupling_map.graph.has_edge(current_layout[v0], current_layout[v1]): + # Accessing layout._v2p directly to avoid overhead from __getitem__ and a + # single access isn't feasible because the layout is updated on each iteration + if self.coupling_map.graph.has_edge( + current_layout._v2p[v0], current_layout._v2p[v1] + ): execute_gate_list.append(node) else: # Single-qubit gates as well as barriers are free execute_gate_list.append(node) @@ -328,8 +335,9 @@ def _obtain_swaps(self, front_layer, current_layout): def _compute_cost(self, layer, layout): cost = 0 + layout_map = layout._v2p for node in layer: - cost += self.coupling_map.distance(layout[node.qargs[0]], layout[node.qargs[1]]) + cost += self.dist_matrix[layout_map[node.qargs[0]], layout_map[node.qargs[1]]] return cost def _score_heuristic(self, heuristic, front_layer, extended_set, layout, swap_qubits=None): @@ -365,7 +373,7 @@ def _transform_gate_for_layout(op_node, layout, device_qreg): mapped_op_node = copy(op_node) premap_qargs = op_node.qargs - mapped_qargs = map(lambda x: device_qreg[layout[x]], premap_qargs) + mapped_qargs = map(lambda x: device_qreg[layout._v2p[x]], premap_qargs) mapped_op_node.qargs = list(mapped_qargs) return mapped_op_node diff --git a/qiskit/transpiler/passes/routing/stochastic_swap.py b/qiskit/transpiler/passes/routing/stochastic_swap.py index 203c3026c056..226228410aae 100644 --- a/qiskit/transpiler/passes/routing/stochastic_swap.py +++ b/qiskit/transpiler/passes/routing/stochastic_swap.py @@ -103,7 +103,7 @@ def run(self, dag): self.seed = np.random.randint(0, np.iinfo(np.int32).max) self.rng = np.random.default_rng(self.seed) logger.debug("StochasticSwap default_rng seeded with seed=%s", self.seed) - + self.coupling_map.compute_distance_matrix() new_dag = self._mapper(dag, self.coupling_map, trials=self.trials) return new_dag @@ -159,7 +159,9 @@ def _layer_permutation(self, layer_partition, layout, qubit_subset, coupling, tr logger.debug("layer_permutation: gates = %s", gates) # Can we already apply the gates? If so, there is no work to do. - dist = sum(coupling.distance(layout[g[0]], layout[g[1]]) for g in gates) + # Accessing via private attributes to avoid overhead from __getitem__ + # and to optimize performance of the distance matrix access + dist = sum(coupling._dist_matrix[layout._v2p[g[0]], layout._v2p[g[1]]] for g in gates) logger.debug("layer_permutation: distance = %s", dist) if dist == len(gates): logger.debug("layer_permutation: nothing to do") @@ -232,8 +234,8 @@ def _layer_permutation(self, layer_partition, layout, qubit_subset, coupling, tr edges = best_edges.edges() for idx in range(best_edges.size // 2): - swap_src = self.trivial_layout[edges[2 * idx]] - swap_tgt = self.trivial_layout[edges[2 * idx + 1]] + swap_src = self.trivial_layout._p2v[edges[2 * idx]] + swap_tgt = self.trivial_layout._p2v[edges[2 * idx + 1]] trial_circuit.apply_operation_back(SwapGate(), [swap_src, swap_tgt], []) best_circuit = trial_circuit diff --git a/qiskit/transpiler/passes/utils/check_map.py b/qiskit/transpiler/passes/utils/check_map.py index 72c2b2c51361..809129ecb86a 100644 --- a/qiskit/transpiler/passes/utils/check_map.py +++ b/qiskit/transpiler/passes/utils/check_map.py @@ -43,10 +43,12 @@ def run(self, dag): """ self.property_set["is_swap_mapped"] = True - if self.coupling_map is None: + if self.coupling_map is None or len(self.coupling_map.graph) == 0: return qubit_indices = {bit: index for index, bit in enumerate(dag.qubits)} + # Use dist matrix directly to avoid validation overhead + dist_matrix = self.coupling_map.distance_matrix for gate in dag.two_qubit_ops(): if dag.has_calibration_for(gate): @@ -54,7 +56,7 @@ def run(self, dag): physical_q0 = qubit_indices[gate.qargs[0]] physical_q1 = qubit_indices[gate.qargs[1]] - if self.coupling_map.distance(physical_q0, physical_q1) != 1: + if dist_matrix[physical_q0, physical_q1] != 1: self.property_set["check_map_msg"] = "{}({}, {}) failed".format( gate.name, physical_q0, diff --git a/qiskit/transpiler/passes/utils/gate_direction.py b/qiskit/transpiler/passes/utils/gate_direction.py index c6fe65de25b8..52a4a50e573a 100644 --- a/qiskit/transpiler/passes/utils/gate_direction.py +++ b/qiskit/transpiler/passes/utils/gate_direction.py @@ -104,6 +104,10 @@ def run(self, dag): cx nodes. """ cmap_edges = set(self.coupling_map.get_edges()) + if not cmap_edges: + return dag + + self.coupling_map.compute_distance_matrix() if len(dag.qregs) > 1: raise TranspilerError( @@ -112,15 +116,17 @@ def run(self, dag): ) trivial_layout = Layout.generate_trivial_layout(*dag.qregs.values()) + layout_map = trivial_layout.get_virtual_bits() + dist_matrix = self.coupling_map.distance_matrix for node in dag.two_qubit_ops(): control = node.qargs[0] target = node.qargs[1] - physical_q0 = trivial_layout[control] - physical_q1 = trivial_layout[target] + physical_q0 = layout_map[control] + physical_q1 = layout_map[target] - if self.coupling_map.distance(physical_q0, physical_q1) != 1: + if dist_matrix[physical_q0, physical_q1] != 1: raise TranspilerError( "The circuit requires a connection between physical " "qubits %s and %s" % (physical_q0, physical_q1) diff --git a/qiskit/transpiler/preset_passmanagers/level2.py b/qiskit/transpiler/preset_passmanagers/level2.py index 20eef266e487..5a492cd06fc5 100644 --- a/qiskit/transpiler/preset_passmanagers/level2.py +++ b/qiskit/transpiler/preset_passmanagers/level2.py @@ -135,7 +135,6 @@ def _trivial_not_perfect(property_set): # layout so we need to clear it before contuing. if property_set["trivial_layout_score"] is not None: if property_set["trivial_layout_score"] != 0: - property_set["layout"]._wrapped = None return True return False diff --git a/qiskit/transpiler/preset_passmanagers/level3.py b/qiskit/transpiler/preset_passmanagers/level3.py index 705546e627e2..8a438bfe4310 100644 --- a/qiskit/transpiler/preset_passmanagers/level3.py +++ b/qiskit/transpiler/preset_passmanagers/level3.py @@ -166,7 +166,6 @@ def _trivial_not_perfect(property_set): # set by trivial layout so we clear that before running CSP if property_set["trivial_layout_score"] is not None: if property_set["trivial_layout_score"] != 0: - property_set["layout"]._wrapped = None return True return False