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

Test round trip of gate to operation to gate #5354

Merged
merged 14 commits into from
May 19, 2022
78 changes: 78 additions & 0 deletions cirq-core/cirq/ops/gate_operation_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down Expand Up @@ -465,3 +468,78 @@ def qubit_index_to_equivalence_group_key(self, index: int) -> int:
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)]
)
Comment on lines +475 to +478
Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't at risk for infinite recursion right ? (if we had two gates with circular dependencies of some kind)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this is fine since the type hierarchy has to be a dag.


# Only test gate subclasses in cirq-core.
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")

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,
# 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
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just loop over gate_subclass less exceptions for clarity sake ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

filename = test_module_spec.test_data_path.joinpath(f"{gate_cls.__name__}.json")
try:
gates = cirq.read_json(filename)
except FileNotFoundError:
if gate_cls in skip_classes:
skipped.add(gate_cls)
continue
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we do an existance check with file.exists instead of a try catch ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

# 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."
)

if not isinstance(gates, collections.abc.Iterable):
gates = [gates]
Comment on lines +532 to +533
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this needed because some json files have lists of things instead of one single thing in them ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep...classic python, object or list of objects :(

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 (
skipped == skip_classes
), "A gate that was supposed to be skipped was not, please update the list of skipped gates."