Skip to content

Commit

Permalink
Make internal Layout and CouplingMap attrs slotted and adjust passes …
Browse files Browse the repository at this point in the history
…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 <[email protected]>

* 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 <[email protected]>

* Update qiskit/transpiler/passes/layout/sabre_layout.py

Co-authored-by: Jake Lishman <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>
Co-authored-by: Jake Lishman <[email protected]>
  • Loading branch information
3 people authored Nov 19, 2021
1 parent 7a13cc8 commit 1567e4e
Show file tree
Hide file tree
Showing 12 changed files with 67 additions and 38 deletions.
24 changes: 14 additions & 10 deletions qiskit/transpiler/coupling.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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:
Expand Down Expand Up @@ -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.
Expand All @@ -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):
Expand Down
4 changes: 3 additions & 1 deletion qiskit/transpiler/layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"""
Expand Down Expand Up @@ -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"
Expand Down
3 changes: 2 additions & 1 deletion qiskit/transpiler/passes/layout/apply_layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
12 changes: 6 additions & 6 deletions qiskit/transpiler/passes/layout/full_ancilla_allocation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
14 changes: 11 additions & 3 deletions qiskit/transpiler/passes/layout/layout_2q_distance.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 1 addition & 3 deletions qiskit/transpiler/passes/layout/sabre_layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
14 changes: 11 additions & 3 deletions qiskit/transpiler/passes/routing/sabre_swap.py
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand All @@ -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.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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
10 changes: 6 additions & 4 deletions qiskit/transpiler/passes/routing/stochastic_swap.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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

Expand Down
6 changes: 4 additions & 2 deletions qiskit/transpiler/passes/utils/check_map.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,20 @@ 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):
continue
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,
Expand Down
12 changes: 9 additions & 3 deletions qiskit/transpiler/passes/utils/gate_direction.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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)
Expand Down
1 change: 0 additions & 1 deletion qiskit/transpiler/preset_passmanagers/level2.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 0 additions & 1 deletion qiskit/transpiler/preset_passmanagers/level3.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 1567e4e

Please sign in to comment.