-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Improve error messages in Target
-based GateDirection
pass
#9787
Changes from 1 commit
9165fcb
6568d9f
607914e
9a0ed3b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,7 +19,7 @@ | |
|
||
from qiskit.converters import dag_to_circuit, circuit_to_dag | ||
from qiskit.circuit import QuantumRegister, ControlFlowOp | ||
from qiskit.dagcircuit import DAGCircuit | ||
from qiskit.dagcircuit import DAGCircuit, DAGOpNode | ||
from qiskit.circuit.library.standard_gates import ( | ||
RYGate, | ||
HGate, | ||
|
@@ -34,6 +34,10 @@ | |
) | ||
|
||
|
||
def _swap_node_qargs(node): | ||
return DAGOpNode(node.op, node.qargs[::-1], node.cargs) | ||
|
||
|
||
class GateDirection(TransformationPass): | ||
"""Modify asymmetric gates to match the hardware coupling direction. | ||
|
||
|
@@ -58,6 +62,8 @@ class GateDirection(TransformationPass): | |
└──────┘ └───┘└──────┘└───┘ | ||
""" | ||
|
||
_KNOWN_REPLACEMENTS = frozenset(["cx", "cz", "ecr", "swap", "rzx", "rxx", "ryy", "rzz"]) | ||
|
||
def __init__(self, coupling_map, target=None): | ||
"""GateDirection pass. | ||
|
||
|
@@ -99,6 +105,8 @@ def __init__(self, coupling_map, target=None): | |
self._swap_dag.add_qreg(qr) | ||
self._swap_dag.apply_operation_back(SwapGate(), [qr[1], qr[0]], []) | ||
|
||
# If adding more replacements (either static or dynamic), also update the class variable | ||
# `_KNOWN_REPLACMENTS` to include them in the error messages. | ||
self._static_replacements = { | ||
"cx": self._cx_dag, | ||
"cz": self._cz_dag, | ||
|
@@ -186,7 +194,7 @@ def _run_coupling_map(self, dag, wire_map, edges=None): | |
else: | ||
raise TranspilerError( | ||
f"Flipping of gate direction is only supported " | ||
f"for {list(self._static_replacements)} at this time, not '{node.name}'." | ||
f"for {list(self._KNOWN_REPLACEMENTS)} at this time, not '{node.name}'." | ||
) | ||
return dag | ||
|
||
|
@@ -277,11 +285,21 @@ def _run_target(self, dag, wire_map): | |
f"The circuit requires a connection between physical qubits {qargs}" | ||
f" for {node.name}" | ||
) | ||
else: | ||
elif self.target.instruction_supported(node.name, qargs): | ||
continue | ||
elif self.target.instruction_supported(node.name, swapped) or dag.has_calibration_for( | ||
_swap_node_qargs(node) | ||
): | ||
raise TranspilerError( | ||
f"Flipping of gate direction is only supported " | ||
f"for {list(self._static_replacements)} at this time, not '{node.name}'." | ||
f"for {list(self._KNOWN_REPLACEMENTS)} at this time, not '{node.name}'." | ||
) | ||
else: | ||
raise TranspilerError( | ||
f"'{node.name}' with parameters '{node.op.params}' is not supported on qubits" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This message could seem bit weird when parameter is empty. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess marginally weird, but still perfectly understandable, right? It'll just say something like
which still looks pretty sensible and easy to parse. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can change it if you'd prefer, though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure how much parameters are important for gate direction. If the user tries slightly different parameter, I don't think he will succeed in transpile. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The point is that a I think it's a bit beyond scope for us to try and pull out all "similar" instructions from the target and try and guess which of those the user might have meant, though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair enough. |
||
f" '{qargs}' in either direction." | ||
) | ||
|
||
return dag | ||
|
||
def run(self, dag): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's worth also mentioning only opposite direction is defined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's kind of implied by the running pass?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pulse level user might be lazy for managing order matching of virtual and logical qubits. So one might hit this error with some typo. Saying having calibration of another direction might help the user to find what's wrong in his code. Otherwise one might start investigating how to implement gate flipping.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is the new message in 607914e for you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I like new one.