-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Give repr to contrib.routing.SwapNetwork, add error message test for unmapped qubits #2251
Changes from all commits
ca5eb5d
44911d0
0eca7c3
9352002
ac37176
fe6a5e5
c28d72d
ef07ac2
d84f57b
ed8bfb3
da209a9
bb73298
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 |
---|---|---|
|
@@ -13,8 +13,8 @@ | |
# limitations under the License. | ||
|
||
import abc | ||
from typing import (Any, cast, Dict, Iterable, Sequence, Tuple, TYPE_CHECKING, | ||
TypeVar, Union, TYPE_CHECKING) | ||
from typing import (Any, Dict, Iterable, Sequence, Tuple, TypeVar, Union, | ||
TYPE_CHECKING) | ||
|
||
from cirq import circuits, ops, optimizers, protocols, value | ||
from cirq.type_workarounds import NotImplementedType | ||
|
@@ -230,9 +230,11 @@ def update_mapping(mapping: Dict[ops.Qid, LogicalIndex], | |
operations: The operations to update according to. | ||
""" | ||
for op in ops.flatten_op_tree(operations): | ||
if (isinstance(op, ops.GateOperation) and | ||
isinstance(op.gate, PermutationGate)): | ||
op.gate.update_mapping(mapping, op.qubits) | ||
# Type check false positive (https://github.com/python/mypy/issues/5374) | ||
gate = ops.op_gate_of_type(op, PermutationGate) # type: ignore | ||
if gate is not None: | ||
# Ignoring type warning about op.qubits not being a tuple. | ||
gate.update_mapping(mapping, op.qubits) # type: ignore | ||
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. Why does mypy think 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 honestly don't know. My guess is that somewhere in the code base a qubits method returns a FrozenSet[Qid] and mypy is inferring the return type from that, but I wasn't able to find where. 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. It seems like a silly thing to spend too much time on, but this seems like a case where mypy might actually be catching an bug, so maybe we shouldn't override it so quickly. |
||
|
||
|
||
def get_logical_operations(operations: 'cirq.OP_TREE', | ||
|
@@ -250,10 +252,11 @@ def get_logical_operations(operations: 'cirq.OP_TREE', | |
qubit. | ||
""" | ||
mapping = initial_mapping.copy() | ||
for op in cast(Iterable['cirq.Operation'], ops.flatten_op_tree(operations)): | ||
if (isinstance(op, ops.GateOperation) and | ||
isinstance(op.gate, PermutationGate)): | ||
op.gate.update_mapping(mapping, op.qubits) | ||
for op in ops.flatten_to_ops(operations): | ||
# Type check false positive (https://github.com/python/mypy/issues/5374) | ||
gate = ops.op_gate_of_type(op, PermutationGate) # type: ignore | ||
if gate is not None: | ||
gate.update_mapping(mapping, op.qubits) | ||
else: | ||
for q in op.qubits: | ||
if mapping.get(q) is None: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -155,3 +155,36 @@ def test_router_bad_args(): | |
cirq.CZ(cirq.LineQubit(i), cirq.LineQubit(i + 1)) for i in range(5)) | ||
with pytest.raises(ValueError): | ||
ccr.route_circuit(circuit, device_graph, algo_name=algo_name) | ||
|
||
|
||
def test_fail_when_operating_on_unmapped_qubits(): | ||
a, b, t = cirq.LineQubit.range(3) | ||
swap = cirq.contrib.acquaintance.SwapPermutationGate() | ||
|
||
# Works before swap. | ||
swap_network = cirq.contrib.routing.SwapNetwork(cirq.Circuit(cirq.CZ(a, t)), | ||
{ | ||
a: a, | ||
b: b | ||
}) | ||
with pytest.raises(ValueError, match='acts on unmapped qubit'): | ||
_ = list(swap_network.get_logical_operations()) | ||
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. Is the mock assignment for pylint? 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. Yes, sometimes pylint complains that the result of an expression is unused. So I got into the habit of assigning to 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. Maybe
It's fine as-is if you have a preference. |
||
|
||
# Works after swap. | ||
swap_network = cirq.contrib.routing.SwapNetwork( | ||
cirq.Circuit(swap(b, t), cirq.CZ(a, b)), { | ||
a: a, | ||
b: b | ||
}) | ||
with pytest.raises(ValueError, match='acts on unmapped qubit'): | ||
_ = list(swap_network.get_logical_operations()) | ||
|
||
# This test case used to cause a CZ(a, a) to be created due to unmapped | ||
# qubits being mapped to stale values. | ||
swap_network = cirq.contrib.routing.SwapNetwork( | ||
cirq.Circuit(swap(a, t), swap(b, t), cirq.CZ(a, b)), { | ||
a: a, | ||
b: b | ||
}) | ||
with pytest.raises(ValueError, match='acts on unmapped qubit'): | ||
_ = list(swap_network.get_logical_operations()) |
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.
Is this just a style preference?
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 can revert it if you prefer. It's just much shorter, and it's why we defined these methods.
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 don't have a preference. IIRC, this is what I originally wrote, and changed it to avoid the type issues.