From f9ce428d6a1a29cd30a73d15ba764c4e85ac0b3c Mon Sep 17 00:00:00 2001 From: Dave Bacon Date: Wed, 30 Mar 2022 09:43:28 -0700 Subject: [PATCH 01/11] Test round trip --- cirq-core/cirq/ops/gate_operation_test.py | 29 +++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/cirq-core/cirq/ops/gate_operation_test.py b/cirq-core/cirq/ops/gate_operation_test.py index 67554537bb5..187dac37e64 100644 --- a/cirq-core/cirq/ops/gate_operation_test.py +++ b/cirq-core/cirq/ops/gate_operation_test.py @@ -11,11 +11,14 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import collections.abc + import numpy as np import pytest import sympy import cirq +import cirq.testing def test_gate_operation_init(): @@ -444,3 +447,29 @@ def _is_parameterized_(self): assert not cirq.is_parameterized(No1().on(q)) assert not cirq.is_parameterized(No2().on(q)) assert cirq.is_parameterized(Yes().on(q)) + + +def test_gate_on_gate(): + def all_subclasses(cls): + return set(cls.__subclasses__()).union( + [s for c in cls.__subclasses__() for s in all_subclasses(c)] + ) + + gate_subclasses = all_subclasses(cirq.Gate) + + test_module_spec = cirq.testing.json.spec_for("cirq.protocols") + + for gate_cls in gate_subclasses: + filename = test_module_spec.test_data_path.joinpath(f"{gate_cls.__name__}.json") + try: + gates = cirq.read_json(filename) + except: + print(gate_cls) + continue + if not isinstance(gates, collections.abc.Iterable): + gates = [gates] + for gate in gates: + if gate.num_qubits(): + qudits = [cirq.LineQid(i, d) for i, d in enumerate(cirq.qid_shape(gate))] + assert gate.on(*qudits).gate == gate + assert False From c779bd76febba9c7ef18d521a3fea02a9b04d55b Mon Sep 17 00:00:00 2001 From: Dave Bacon Date: Thu, 12 May 2022 00:02:51 +0000 Subject: [PATCH 02/11] Add test to round trip gate to operation to gate --- cirq-core/cirq/ops/gate_operation_test.py | 107 +++++++++++++++++----- 1 file changed, 82 insertions(+), 25 deletions(-) diff --git a/cirq-core/cirq/ops/gate_operation_test.py b/cirq-core/cirq/ops/gate_operation_test.py index 4084c1419a8..a303ffeddfb 100644 --- a/cirq-core/cirq/ops/gate_operation_test.py +++ b/cirq-core/cirq/ops/gate_operation_test.py @@ -449,8 +449,28 @@ def _is_parameterized_(self): assert cirq.is_parameterized(Yes().on(q)) -<<<<<<< HEAD -def test_gate_on_gate(): +def test_group_interchangeable_qubits_creates_tuples_with_unique_keys(): + class MyGate(cirq.Gate, cirq.InterchangeableQubitsGate): + def __init__(self, num_qubits) -> None: + self._num_qubits = num_qubits + + def num_qubits(self) -> int: + return self._num_qubits + + def qubit_index_to_equivalence_group_key(self, index: int) -> int: + if index % 2 == 0: + return index + return 0 + + qubits = cirq.LineQubit.range(4) + gate = MyGate(len(qubits)) + + assert gate(qubits[0], qubits[1], qubits[2], qubits[3]) == gate( + qubits[3], qubits[1], qubits[2], qubits[0] + ) + + +def test_gate_to_operation_to_gate_round_trips(): def all_subclasses(cls): return set(cls.__subclasses__()).union( [s for c in cls.__subclasses__() for s in all_subclasses(c)] @@ -460,37 +480,74 @@ def all_subclasses(cls): test_module_spec = cirq.testing.json.spec_for("cirq.protocols") + skip_classes = { + # Abstract or private parent classes. + cirq.BaseDensePauliString, + cirq.EigenGate, + cirq.Pauli, + # Private gates. + cirq.transformers.analytical_decompositions.two_qubit_to_fsim._BGate, + cirq.ops.raw_types._InverseCompositeGate, + cirq.circuits.qasm_output.QasmTwoQubitGate, + cirq.circuits.quil_output.QuilTwoQubitGate, + cirq.circuits.quil_output.QuilOneQubitGate, + cirq.ion.ion_gates.MSGate, + # Gate features. + cirq.SingleQubitGate, + # Testing gate features. + cirq.testing.TwoQubitGate, + cirq.testing.ThreeQubitGate, + cirq.testing.SingleQubitGate, + # Contrib gates + cirq.contrib.acquaintance.SwapNetworkGate, + cirq.contrib.acquaintance.ShiftSwapNetworkGate, + cirq.contrib.acquaintance.permutation.MappingDisplayGate, + cirq.contrib.acquaintance.CircularShiftGate, + cirq.contrib.acquaintance.BipartiteSwapNetworkGate, + cirq.contrib.acquaintance.SwapPermutationGate, + cirq.contrib.acquaintance.AcquaintanceOpportunityGate, + cirq.contrib.acquaintance.PermutationGate, + cirq.contrib.acquaintance.LinearPermutationGate, + # Interop gates + cirq.interop.quirk.QuirkQubitPermutationGate, + cirq.interop.quirk.QuirkArithmeticGate, + # No reason given for missing json. + # TODO(#5353): Serialize these gates. + cirq.DiagonalGate, + cirq.TwoQubitDiagonalGate, + cirq.ThreeQubitDiagonalGate, + cirq.PauliInteractionGate, + cirq.ArithmeticGate, + } + + # Gates that do not satisfy the contract. + # TODO(#5167): Fix this case. + exceptions = {cirq.PauliStringPhasorGate} + + skipped = set() for gate_cls in gate_subclasses: + if gate_cls in exceptions: + continue filename = test_module_spec.test_data_path.joinpath(f"{gate_cls.__name__}.json") try: gates = cirq.read_json(filename) - except: - print(gate_cls) - continue + except FileNotFoundError: + if gate_cls in skip_classes: + skipped.add(gate_cls) + continue + raise AssertionError( + f"{gate_cls} has no json file, please add a json file or add to the list of " + "classes to be skipped if there is a reason this gate should not round trip " + "to a gate via creating an operation." + ) + if not isinstance(gates, collections.abc.Iterable): gates = [gates] for gate in gates: if gate.num_qubits(): qudits = [cirq.LineQid(i, d) for i, d in enumerate(cirq.qid_shape(gate))] assert gate.on(*qudits).gate == gate - assert False - -def test_group_interchangeable_qubits_creates_tuples_with_unique_keys(): - class MyGate(cirq.Gate, cirq.InterchangeableQubitsGate): - def __init__(self, num_qubits) -> None: - self._num_qubits = num_qubits - - def num_qubits(self) -> int: - return self._num_qubits - - def qubit_index_to_equivalence_group_key(self, index: int) -> int: - if index % 2 == 0: - return index - return 0 - qubits = cirq.LineQubit.range(4) - gate = MyGate(len(qubits)) - - assert gate(qubits[0], qubits[1], qubits[2], qubits[3]) == gate( - qubits[3], qubits[1], qubits[2], qubits[0] - ) + assert ( + skipped == skip_classes + ), "A gate that was supposed to be skipped was not, please update the list of skipped gates." From 3064864dc39041af74c66ccffad218f63a11876d Mon Sep 17 00:00:00 2001 From: Dave Bacon Date: Thu, 12 May 2022 00:16:29 +0000 Subject: [PATCH 03/11] Fix --- cirq-core/cirq/ops/gate_operation_test.py | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/cirq-core/cirq/ops/gate_operation_test.py b/cirq-core/cirq/ops/gate_operation_test.py index a303ffeddfb..34755a268c2 100644 --- a/cirq-core/cirq/ops/gate_operation_test.py +++ b/cirq-core/cirq/ops/gate_operation_test.py @@ -476,7 +476,8 @@ def all_subclasses(cls): [s for c in cls.__subclasses__() for s in all_subclasses(c)] ) - gate_subclasses = all_subclasses(cirq.Gate) + # Filter out contrib, since we should decouple cirq from contrib. + gate_subclasses = [g for g in all_subclasses(cirq.Gate) if "contrib" not in g.__module__] test_module_spec = cirq.testing.json.spec_for("cirq.protocols") @@ -498,16 +499,6 @@ def all_subclasses(cls): cirq.testing.TwoQubitGate, cirq.testing.ThreeQubitGate, cirq.testing.SingleQubitGate, - # Contrib gates - cirq.contrib.acquaintance.SwapNetworkGate, - cirq.contrib.acquaintance.ShiftSwapNetworkGate, - cirq.contrib.acquaintance.permutation.MappingDisplayGate, - cirq.contrib.acquaintance.CircularShiftGate, - cirq.contrib.acquaintance.BipartiteSwapNetworkGate, - cirq.contrib.acquaintance.SwapPermutationGate, - cirq.contrib.acquaintance.AcquaintanceOpportunityGate, - cirq.contrib.acquaintance.PermutationGate, - cirq.contrib.acquaintance.LinearPermutationGate, # Interop gates cirq.interop.quirk.QuirkQubitPermutationGate, cirq.interop.quirk.QuirkArithmeticGate, From 16e6714c75a547018584b1ffa53fadeca0cc6e9d Mon Sep 17 00:00:00 2001 From: Dave Bacon Date: Thu, 12 May 2022 00:30:31 +0000 Subject: [PATCH 04/11] only stuff in core --- cirq-core/cirq/ops/gate_operation_test.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cirq-core/cirq/ops/gate_operation_test.py b/cirq-core/cirq/ops/gate_operation_test.py index 34755a268c2..b516646aebc 100644 --- a/cirq-core/cirq/ops/gate_operation_test.py +++ b/cirq-core/cirq/ops/gate_operation_test.py @@ -477,7 +477,11 @@ def all_subclasses(cls): ) # Filter out contrib, since we should decouple cirq from contrib. - gate_subclasses = [g for g in all_subclasses(cirq.Gate) if "contrib" not in g.__module__] + gate_subclasses = [ + g + for g in all_subclasses(cirq.Gate) + if "cirq." in g.__module__ and "contrib" not in g.__module__ + ] test_module_spec = cirq.testing.json.spec_for("cirq.protocols") From ccdbea5246788c175a7c3c76a974a17b0ed24cf3 Mon Sep 17 00:00:00 2001 From: Dave Bacon Date: Thu, 12 May 2022 00:32:29 +0000 Subject: [PATCH 05/11] fix --- cirq-core/cirq/ops/gate_operation_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cirq-core/cirq/ops/gate_operation_test.py b/cirq-core/cirq/ops/gate_operation_test.py index b516646aebc..ecc5c8eacf4 100644 --- a/cirq-core/cirq/ops/gate_operation_test.py +++ b/cirq-core/cirq/ops/gate_operation_test.py @@ -476,7 +476,7 @@ def all_subclasses(cls): [s for c in cls.__subclasses__() for s in all_subclasses(c)] ) - # Filter out contrib, since we should decouple cirq from contrib. + # Only test gate subclasses in cirq-core. gate_subclasses = [ g for g in all_subclasses(cirq.Gate) From 768109fc3fec16b2ab12acdfc8db5fcb1ae492d4 Mon Sep 17 00:00:00 2001 From: Dave Bacon Date: Thu, 12 May 2022 03:22:00 +0000 Subject: [PATCH 06/11] exclude test --- cirq-core/cirq/ops/gate_operation_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cirq-core/cirq/ops/gate_operation_test.py b/cirq-core/cirq/ops/gate_operation_test.py index ecc5c8eacf4..a9cc99f9d39 100644 --- a/cirq-core/cirq/ops/gate_operation_test.py +++ b/cirq-core/cirq/ops/gate_operation_test.py @@ -480,7 +480,7 @@ def all_subclasses(cls): gate_subclasses = [ g for g in all_subclasses(cirq.Gate) - if "cirq." in g.__module__ and "contrib" not in g.__module__ + if "cirq." in g.__module__ and "contrib" not in g.__module__ and "_test" not in g.__module__ ] test_module_spec = cirq.testing.json.spec_for("cirq.protocols") From 9a515eae44b1b291dc2908769cef21081e5a72db Mon Sep 17 00:00:00 2001 From: Dave Bacon Date: Thu, 12 May 2022 03:22:07 +0000 Subject: [PATCH 07/11] exclude test --- cirq-core/cirq/ops/gate_operation_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cirq-core/cirq/ops/gate_operation_test.py b/cirq-core/cirq/ops/gate_operation_test.py index a9cc99f9d39..d03b1963b91 100644 --- a/cirq-core/cirq/ops/gate_operation_test.py +++ b/cirq-core/cirq/ops/gate_operation_test.py @@ -480,7 +480,7 @@ def all_subclasses(cls): gate_subclasses = [ g for g in all_subclasses(cirq.Gate) - if "cirq." in g.__module__ and "contrib" not in g.__module__ and "_test" not in g.__module__ + if "cirq." in g.__module__ and "contrib" not in g.__module__ and "test" not in g.__module__ ] test_module_spec = cirq.testing.json.spec_for("cirq.protocols") From b64d8eaf27f4bb138b23b29bb652763434088b0f Mon Sep 17 00:00:00 2001 From: Dave Bacon Date: Thu, 12 May 2022 03:39:03 +0000 Subject: [PATCH 08/11] remove test from skipped --- cirq-core/cirq/ops/gate_operation_test.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/cirq-core/cirq/ops/gate_operation_test.py b/cirq-core/cirq/ops/gate_operation_test.py index d03b1963b91..1a178cdfc6c 100644 --- a/cirq-core/cirq/ops/gate_operation_test.py +++ b/cirq-core/cirq/ops/gate_operation_test.py @@ -499,10 +499,6 @@ def all_subclasses(cls): cirq.ion.ion_gates.MSGate, # Gate features. cirq.SingleQubitGate, - # Testing gate features. - cirq.testing.TwoQubitGate, - cirq.testing.ThreeQubitGate, - cirq.testing.SingleQubitGate, # Interop gates cirq.interop.quirk.QuirkQubitPermutationGate, cirq.interop.quirk.QuirkArithmeticGate, From 114fedaabaab968fe21d624ac9cf442d68245aa1 Mon Sep 17 00:00:00 2001 From: Dave Bacon Date: Thu, 12 May 2022 04:19:09 +0000 Subject: [PATCH 09/11] ignore coverage in test --- cirq-core/cirq/ops/gate_operation_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cirq-core/cirq/ops/gate_operation_test.py b/cirq-core/cirq/ops/gate_operation_test.py index 1a178cdfc6c..574346fa921 100644 --- a/cirq-core/cirq/ops/gate_operation_test.py +++ b/cirq-core/cirq/ops/gate_operation_test.py @@ -530,7 +530,7 @@ def all_subclasses(cls): f"{gate_cls} has no json file, please add a json file or add to the list of " "classes to be skipped if there is a reason this gate should not round trip " "to a gate via creating an operation." - ) + ) # coverage:ignore if not isinstance(gates, collections.abc.Iterable): gates = [gates] From e7dd8c8b2102b474231f6e8d140e1ee7b4b8b213 Mon Sep 17 00:00:00 2001 From: Dave Bacon Date: Thu, 12 May 2022 16:28:49 +0000 Subject: [PATCH 10/11] move coverage comment --- cirq-core/cirq/ops/gate_operation_test.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cirq-core/cirq/ops/gate_operation_test.py b/cirq-core/cirq/ops/gate_operation_test.py index 574346fa921..a885a52cb44 100644 --- a/cirq-core/cirq/ops/gate_operation_test.py +++ b/cirq-core/cirq/ops/gate_operation_test.py @@ -526,11 +526,12 @@ def all_subclasses(cls): if gate_cls in skip_classes: skipped.add(gate_cls) continue + # coverage:ignore raise AssertionError( f"{gate_cls} has no json file, please add a json file or add to the list of " "classes to be skipped if there is a reason this gate should not round trip " "to a gate via creating an operation." - ) # coverage:ignore + ) if not isinstance(gates, collections.abc.Iterable): gates = [gates] From 60ec26654bffcb590e41ebaef3b6fa810b12bb0c Mon Sep 17 00:00:00 2001 From: Dave Bacon Date: Thu, 19 May 2022 16:46:13 +0000 Subject: [PATCH 11/11] reviewer comments --- cirq-core/cirq/ops/gate_operation_test.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/cirq-core/cirq/ops/gate_operation_test.py b/cirq-core/cirq/ops/gate_operation_test.py index a885a52cb44..3352694c045 100644 --- a/cirq-core/cirq/ops/gate_operation_test.py +++ b/cirq-core/cirq/ops/gate_operation_test.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. import collections.abc +import pathlib import numpy as np import pytest @@ -477,11 +478,11 @@ def all_subclasses(cls): ) # Only test gate subclasses in cirq-core. - gate_subclasses = [ + gate_subclasses = { g for g in all_subclasses(cirq.Gate) if "cirq." in g.__module__ and "contrib" not in g.__module__ and "test" not in g.__module__ - ] + } test_module_spec = cirq.testing.json.spec_for("cirq.protocols") @@ -504,9 +505,6 @@ def all_subclasses(cls): cirq.interop.quirk.QuirkArithmeticGate, # No reason given for missing json. # TODO(#5353): Serialize these gates. - cirq.DiagonalGate, - cirq.TwoQubitDiagonalGate, - cirq.ThreeQubitDiagonalGate, cirq.PauliInteractionGate, cirq.ArithmeticGate, } @@ -516,13 +514,11 @@ def all_subclasses(cls): exceptions = {cirq.PauliStringPhasorGate} skipped = set() - for gate_cls in gate_subclasses: - if gate_cls in exceptions: - continue + for gate_cls in gate_subclasses - exceptions: filename = test_module_spec.test_data_path.joinpath(f"{gate_cls.__name__}.json") - try: + if pathlib.Path(filename).is_file(): gates = cirq.read_json(filename) - except FileNotFoundError: + else: if gate_cls in skip_classes: skipped.add(gate_cls) continue