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

Fix bug in cirq-ft due to which T-complexity fails for gates with an empty decomposition / no-op #6252

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion cirq-ft/cirq_ft/algos/select_swap_qrom.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ def __init__(
assert len(target_bitsizes) == len(data)
assert all(t >= max(d).bit_length() for t, d in zip(target_bitsizes, data))
self._num_sequences = len(data)
self._target_bitsizes = target_bitsizes
self._target_bitsizes = tuple(target_bitsizes)
self._iteration_length = len(data[0])
if block_size is None:
# Figure out optimal value of block_size
Expand Down
6 changes: 6 additions & 0 deletions cirq-ft/cirq_ft/algos/select_swap_qrom_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,3 +101,9 @@ def test_qroam_diagram():
def test_qroam_raises():
with pytest.raises(ValueError, match="must be of equal length"):
_ = cirq_ft.SelectSwapQROM([1, 2], [1, 2, 3])


def test_qroam_hashable():
qrom = cirq_ft.SelectSwapQROM([1, 2, 5, 6, 7, 8])
assert hash(qrom) is not None
assert cirq_ft.t_complexity(qrom) == cirq_ft.TComplexity(32, 160, 0)
2 changes: 1 addition & 1 deletion cirq-ft/cirq_ft/infra/decompose_protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,4 +98,4 @@ def _decompose_once_considering_known_decomposition(val: Any) -> DecomposeResult
else:
decomposed = cirq.decompose_once(val, context=context, flatten=False, default=None)

return [*cirq.flatten_to_ops(decomposed)] if decomposed else None
return [*cirq.flatten_to_ops(decomposed)] if decomposed is not None else None
15 changes: 14 additions & 1 deletion cirq-ft/cirq_ft/infra/decompose_protocol_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@
import cirq
import numpy as np
import pytest
from cirq_ft.infra.decompose_protocol import _fredkin, _try_decompose_from_known_decompositions
from cirq_ft.infra.decompose_protocol import (
_fredkin,
_try_decompose_from_known_decompositions,
_decompose_once_considering_known_decomposition,
)


def test_fredkin_unitary():
Expand Down Expand Up @@ -43,3 +47,12 @@ def test_decompose_fredkin(gate):
for o in cirq.flatten_op_tree(_fredkin((c, t1, t2), context))
)
assert want == _try_decompose_from_known_decompositions(op, context)


def test_known_decomposition_empty_unitary():
class DecomposeEmptyList(cirq.testing.SingleQubitGate):
def _decompose_(self, _):
return []
Copy link
Collaborator

Choose a reason for hiding this comment

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

check if it's pass as well. _decompose_ is usually a generator

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It does pass when _decompose_ is an empty generator - the test for cirq_ft.SelectSwapQROM exercises that path. Also, a decompose method with only pass will not be a generator, so we'll have to do something like if False: yield [], which would be awkward.


gate = DecomposeEmptyList()
assert _decompose_once_considering_known_decomposition(gate) == []