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

Update GateDirection and checker for control flow #8822

Merged
merged 3 commits into from
Oct 1, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
62 changes: 42 additions & 20 deletions qiskit/transpiler/passes/utils/check_gate_direction.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@

"""Check if the gates follow the right direction with respect to the coupling map."""

from qiskit.transpiler.layout import Layout
from qiskit.circuit import ControlFlowOp
from qiskit.converters import circuit_to_dag
from qiskit.transpiler.basepasses import AnalysisPass


Expand All @@ -33,6 +34,40 @@ def __init__(self, coupling_map, target=None):
self.coupling_map = coupling_map
self.target = target

def _coupling_map_visit(self, dag, wire_map, edges=None):
if edges is None:
edges = self.coupling_map.get_edges()
for node in dag.two_qubit_ops():
if isinstance(node.op, ControlFlowOp):
continue
if (wire_map[node.qargs[0]], wire_map[node.qargs[1]]) not in edges:
return False
for node in dag.op_nodes(ControlFlowOp):
for block in node.op.blocks:
inner_wire_map = {
inner: wire_map[outer] for outer, inner in zip(node.qargs, block.qubits)
}
if not self._coupling_map_visit(circuit_to_dag(block), inner_wire_map, edges):
return False
jakelishman marked this conversation as resolved.
Show resolved Hide resolved
return True

def _target_visit(self, dag, wire_map):
for node in dag.two_qubit_ops():
if isinstance(node.op, ControlFlowOp):
continue
if not self.target.instruction_supported(
node.op.name, (wire_map[node.qargs[0]], wire_map[node.qargs[1]])
):
return False
for node in dag.op_nodes(ControlFlowOp):
for block in node.op.blocks:
inner_wire_map = {
inner: wire_map[outer] for outer, inner in zip(node.qargs, block.qubits)
}
if not self._target_visit(circuit_to_dag(block), inner_wire_map):
return False
jakelishman marked this conversation as resolved.
Show resolved Hide resolved
return True

def run(self, dag):
"""Run the CheckGateDirection pass on `dag`.

Expand All @@ -42,22 +77,9 @@ def run(self, dag):
Args:
dag (DAGCircuit): DAG to check.
"""
self.property_set["is_direction_mapped"] = True
edges = self.coupling_map.get_edges()
trivial_layout = Layout.generate_trivial_layout(*dag.qregs.values())
if self.target is None:
for gate in dag.two_qubit_ops():
physical_q0 = trivial_layout[gate.qargs[0]]
physical_q1 = trivial_layout[gate.qargs[1]]

if (physical_q0, physical_q1) not in edges:
self.property_set["is_direction_mapped"] = False
return
else:
for gate in dag.two_qubit_ops():
physical_q0 = trivial_layout[gate.qargs[0]]
physical_q1 = trivial_layout[gate.qargs[1]]

if (physical_q0, physical_q1) not in self.target[gate.op.name]:
self.property_set["is_direction_mapped"] = False
return
wire_map = {bit: i for i, bit in enumerate(dag.qubits)}
self.property_set["is_direction_mapped"] = (
self._coupling_map_visit(dag, wire_map)
if self.target is None
else self._target_visit(dag, wire_map)
)
188 changes: 91 additions & 97 deletions qiskit/transpiler/passes/utils/gate_direction.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@

from math import pi

from qiskit.transpiler.layout import Layout
from qiskit.transpiler.basepasses import TransformationPass
from qiskit.transpiler.exceptions import TranspilerError

from qiskit.circuit import QuantumRegister
from qiskit.converters import dag_to_circuit, circuit_to_dag
from qiskit.circuit import QuantumRegister, ControlFlowOp
from qiskit.dagcircuit import DAGCircuit
from qiskit.circuit.library.standard_gates import RYGate, HGate, CXGate, CZGate, ECRGate, RZXGate

Expand Down Expand Up @@ -83,6 +83,8 @@ def __init__(self, coupling_map, target=None):
self._cz_dag.add_qreg(qr)
self._cz_dag.apply_operation_back(CZGate(), [qr[1], qr[0]], [])

self._static_replacements = {"cx": self._cx_dag, "cz": self._cz_dag, "ecr": self._ecr_dag}

@staticmethod
def _rzx_dag(parameter):
_rzx_dag = DAGCircuit()
Expand All @@ -95,6 +97,90 @@ def _rzx_dag(parameter):
_rzx_dag.apply_operation_back(HGate(), [qr[1]], [])
return _rzx_dag

def _run_coupling_map(self, dag, wire_map, edges=None):
if edges is None:
edges = set(self.coupling_map.get_edges())
if not edges:
return dag
for node in dag.two_qubit_ops():
if isinstance(node.op, ControlFlowOp):
continue
qargs = (wire_map[node.qargs[0]], wire_map[node.qargs[1]])
if qargs not in edges and (qargs[1], qargs[0]) not in edges:
raise TranspilerError(
f"The circuit requires a connection between physical qubits {qargs}"
)
if qargs not in edges:
replacement = self._static_replacements.get(node.name)
if replacement is not None:
dag.substitute_node_with_dag(node, replacement)
elif node.name == "rzx":
dag.substitute_node_with_dag(node, self._rzx_dag(*node.op.params))
else:
raise TranspilerError(
f"Flipping of gate direction is only supported "
f"for {list(self._static_replacements)} at this time, not '{node.name}'."
)
for node in dag.op_nodes(ControlFlowOp):
node.op = node.op.replace_blocks(
dag_to_circuit(
self._run_coupling_map(
circuit_to_dag(block),
{inner: wire_map[outer] for outer, inner in zip(node.qargs, block.qubits)},
edges,
)
)
for block in node.op.blocks
)
return dag

def _run_target(self, dag, wire_map):
for node in dag.two_qubit_ops():
if isinstance(node.op, ControlFlowOp):
continue
qargs = (wire_map[node.qargs[0]], wire_map[node.qargs[1]])
swapped = (qargs[1], qargs[0])
if node.name in self._static_replacements:
if self.target.instruction_supported(node.name, qargs):
continue
if self.target.instruction_supported(node.name, swapped):
dag.substitute_node_with_dag(node, self._static_replacements[node.name])
else:
raise TranspilerError(
f"The circuit requires a connection between physical qubits {qargs}"
f" for {node.name}"
)
elif node.name == "rzx":
if self.target.instruction_supported(
qargs=qargs, operation_class=RZXGate, parameters=node.op.params
):
continue
if self.target.instruction_supported(
qargs=swapped, operation_class=RZXGate, parameters=node.op.params
):
dag.substitute_node_with_dag(node, self._rzx_dag(*node.op.params))
else:
raise TranspilerError(
f"The circuit requires a connection between physical qubits {qargs}"
f" for {node.name}"
Comment on lines +173 to +185
Copy link
Member

Choose a reason for hiding this comment

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

Ooh, this is nice, thanks for fixing the TODO there! @ajavadia will be glad to see this because it pushes us closer to being able to enable https://arxiv.org/abs/2111.02535 by default in transpile() if the backend supports tuned variants of rzx.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh yeah - since Target.instruction_supported got added, the todo became very easy to action. I think we're probably leaving some efficiency on the table here when it's called in a loop with operation_class set, but that's for another day most likely.

)
else:
raise TranspilerError(
f"Flipping of gate direction is only supported "
f"for {list(self._static_replacements)} at this time, not '{node.name}'."
)
for node in dag.op_nodes(ControlFlowOp):
node.op = node.op.replace_blocks(
dag_to_circuit(
self._run_target(
circuit_to_dag(block),
{inner: wire_map[outer] for outer, inner in zip(node.qargs, block.qubits)},
)
)
for block in node.op.blocks
)
return dag

def run(self, dag):
"""Run the GateDirection pass on `dag`.

Expand All @@ -111,104 +197,12 @@ def run(self, dag):
TranspilerError: If the circuit cannot be mapped just by flipping the
cx nodes.
"""
trivial_layout = Layout.generate_trivial_layout(*dag.qregs.values())
layout_map = trivial_layout.get_virtual_bits()
layout_map = {bit: i for i, bit in enumerate(dag.qubits)}
if len(dag.qregs) > 1:
raise TranspilerError(
"GateDirection expects a single qreg input DAG,"
"but input DAG had qregs: {}.".format(dag.qregs)
)
if self.target is None:
cmap_edges = set(self.coupling_map.get_edges())
if not cmap_edges:
return dag

self.coupling_map.compute_distance_matrix()

dist_matrix = self.coupling_map.distance_matrix

for node in dag.two_qubit_ops():
control = node.qargs[0]
target = node.qargs[1]

physical_q0 = layout_map[control]
physical_q1 = layout_map[target]

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)
)

if (physical_q0, physical_q1) not in cmap_edges:
if node.name == "cx":
dag.substitute_node_with_dag(node, self._cx_dag)
elif node.name == "cz":
dag.substitute_node_with_dag(node, self._cz_dag)
elif node.name == "ecr":
dag.substitute_node_with_dag(node, self._ecr_dag)
elif node.name == "rzx":
dag.substitute_node_with_dag(node, self._rzx_dag(*node.op.params))
else:
raise TranspilerError(
f"Flipping of gate direction is only supported "
f"for CX, ECR, and RZX at this time, not {node.name}."
)
else:
# TODO: Work with the gate instances and only use names as look up keys.
# This will require iterating over the target names to build a mapping
# of names to gates that implement CXGate, ECRGate, RZXGate (including
# fixed angle variants)
for node in dag.two_qubit_ops():
control = node.qargs[0]
target = node.qargs[1]

physical_q0 = layout_map[control]
physical_q1 = layout_map[target]

if node.name == "cx":
if (physical_q0, physical_q1) in self.target["cx"]:
continue
if (physical_q1, physical_q0) in self.target["cx"]:
dag.substitute_node_with_dag(node, self._cx_dag)
else:
raise TranspilerError(
"The circuit requires a connection between physical "
"qubits %s and %s for cx" % (physical_q0, physical_q1)
)
elif node.name == "cz":
if (physical_q0, physical_q1) in self.target["cz"]:
continue
if (physical_q1, physical_q0) in self.target["cz"]:
dag.substitute_node_with_dag(node, self._cz_dag)
else:
raise TranspilerError(
"The circuit requires a connection between physical "
"qubits %s and %s for cz" % (physical_q0, physical_q1)
)
elif node.name == "ecr":
if (physical_q0, physical_q1) in self.target["ecr"]:
continue
if (physical_q1, physical_q0) in self.target["ecr"]:
dag.substitute_node_with_dag(node, self._ecr_dag)
else:
raise TranspilerError(
"The circuit requires a connection between physical "
"qubits %s and %s for ecr" % (physical_q0, physical_q1)
)
elif node.name == "rzx":
if (physical_q0, physical_q1) in self.target["rzx"]:
continue
if (physical_q1, physical_q0) in self.target["rzx"]:
dag.substitute_node_with_dag(node, self._rzx_dag(*node.op.params))
else:
raise TranspilerError(
"The circuit requires a connection between physical "
"qubits %s and %s for rzx" % (physical_q0, physical_q1)
)
else:
raise TranspilerError(
f"Flipping of gate direction is only supported "
f"for CX, ECR, and RZX at this time, not {node.name}."
)
return dag
return self._run_coupling_map(dag, layout_map)
return self._run_target(dag, layout_map)
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
fixes:
- |
The :class:`.GateDirection` transpiler pass will now respect the available
values for gate parameters when handling parametrised gates with a
:class:`.Target`.
Loading