Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make internal Layout and CouplingMap attrs slotted and adjust passes for fast access #7036

Merged
merged 25 commits into from
Nov 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
09f2486
Make internal layout and coupling map attributes public and slotted
mtreinish Sep 17, 2021
8e15742
Update pass access patterns for performance
mtreinish Sep 17, 2021
15e90e2
Move connectivity check inside distance matrix rebuild condition
mtreinish Sep 20, 2021
9268df5
Adjust array access pattern to avoid temporary object contruction
mtreinish Sep 20, 2021
550e8c6
Merge branch 'main' into tune-layout-coupling
mtreinish Sep 20, 2021
e36efe3
Merge remote-tracking branch 'origin/main' into tune-layout-coupling
mtreinish Oct 26, 2021
adfe5ca
Make attributes private again
mtreinish Oct 26, 2021
2f6d836
Update sabre layout virtual bit usage
mtreinish Oct 26, 2021
b7d7171
Merge branch 'main' into tune-layout-coupling
mtreinish Oct 26, 2021
73d541a
Apply suggestions from code review
mtreinish Nov 3, 2021
0154230
Get mapping from layout once outside of loop
mtreinish Nov 3, 2021
bc92386
Merge remote-tracking branch 'origin/main' into tune-layout-coupling
mtreinish Nov 3, 2021
6809acf
Update qiskit/transpiler/passes/layout/sabre_layout.py
mtreinish Nov 18, 2021
b6bed17
Merge remote-tracking branch 'origin/main' into tune-layout-coupling
mtreinish Nov 18, 2021
0fc21a2
Locally cache objects in compute_cost
mtreinish Nov 18, 2021
4e4da2b
Adjust access in full ancilla and layout2q passes to avoid private ac…
mtreinish Nov 19, 2021
bdba278
Merge branch 'main' into tune-layout-coupling
mtreinish Nov 19, 2021
adccd4f
Adjust gate_direction to limit private access
mtreinish Nov 19, 2021
013c804
Add comments about private access to Checkmap
mtreinish Nov 19, 2021
2e1db9c
Update qiskit/transpiler/passes/layout/layout_2q_distance.py
mtreinish Nov 19, 2021
95caec8
Updates to sabre passes
mtreinish Nov 19, 2021
2ada462
Merge branch 'main' into tune-layout-coupling
mtreinish Nov 19, 2021
009b005
Remove unecessary private access from stochastic swap
mtreinish Nov 19, 2021
8046f43
Add comment on private access to stochastic swap
mtreinish Nov 19, 2021
657d605
Adjust distance matrix access
mtreinish Nov 19, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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]
):
mtreinish marked this conversation as resolved.
Show resolved Hide resolved
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)
kdk marked this conversation as resolved.
Show resolved Hide resolved
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These things are (presumably) part of the old FencedObject stuff that appeared at the start of the transpiler. I'm guessing it's being removed because the Layout became slotted and you didn't want to add the extra slot? Do we need the FencedPropertySet stuff at all really - is it useful for error checking?

Copy link
Member Author

@mtreinish mtreinish Oct 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I removed this because slots meant I couldn't add an arbitrary attribute to the Layout object and it didn't seem worth the slot especially since it wasn't universally used.

I think we can look into deprecating the FencedPropertySet. At one time it was used to ensure that a TransformationPass could not modify the property set but that restriction was removed in #4387 so I'm not sure anything needs it anymore.

(edit: looking at the running passmanager object the flow controller gets a fenced property set to prevent a condition callable from modifying it. I'm not sure how important that is in practice, I've never seen an error like that come up before)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds sensible to me!

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