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

change type annotation from float to TParamVal for theta and phi #4266

Closed
Closed
Show file tree
Hide file tree
Changes from 2 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-core/cirq/ops/diagonal_gate.py
Expand Up @@ -144,7 +144,7 @@ def _value_equality_values_(self) -> Any:
return tuple(self._diag_angles_radians)

def _decompose_for_basis(
self, index: int, bit_flip: int, theta: float, qubits: Sequence['cirq.Qid']
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we should make the changes here since below _decompose_ explicitly checks that self should not be parameterized and hence the parameter will always be a float here.

self, index: int, bit_flip: int, theta: value.TParamVal, qubits: Sequence['cirq.Qid']
) -> Iterator[Union['cirq.ZPowGate', 'cirq.CXPowGate']]:
if index == 0:
return []
Expand Down
2 changes: 1 addition & 1 deletion cirq-core/cirq/ops/fsim_gate.py
Expand Up @@ -78,7 +78,7 @@ class FSimGate(gate_features.TwoQubitGate, gate_features.InterchangeableQubitsGa
FSimGate(θ, φ) = ISWAP**(-2θ/π) CZPowGate(exponent=-φ/π)
"""

def __init__(self, theta: float, phi: float) -> None:
def __init__(self, theta: value.TParamVal, phi: value.TParamVal) -> None:
"""
Args:
theta: Swap angle on the ``|01⟩`` ``|10⟩`` subspace, in radians.
Expand Down
4 changes: 2 additions & 2 deletions cirq-google/cirq_google/calibration/workflow_test.py
Expand Up @@ -1181,7 +1181,7 @@ def test_run_floquet_characterization_for_moments():
itertools.product([0.1, 0.7], [-0.3, 0.1, 0.5], [-0.3, 0.2, 0.4], [-0.6, 0.1, 0.6], [0.2, 0.6]),
)
def test_fsim_phase_corrections(
theta: float, zeta: float, chi: float, gamma: float, phi: float
theta: cirq.value.TParamVal, zeta: float, chi: float, gamma: float, phi: cirq.value.TParamVal
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is available on the main cirq namespace, so can just do

Suggested change
theta: cirq.value.TParamVal, zeta: float, chi: float, gamma: float, phi: cirq.value.TParamVal
theta: cirq.TParamVal, zeta: float, chi: float, gamma: float, phi: cirq.TParamVal

Can do this throughout cirq_google, since it's external to cirq_core.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see a reason why only theta / phi should accept TParamVal here. It's not needed for both since we compute the unitary of the expected_gate in the function and hence not using any parameters. Also, it doesn't look consistent with other parameters (zeta, chi, gamma).

) -> None:
a, b = cirq.LineQubit.range(2)

Expand Down Expand Up @@ -1209,7 +1209,7 @@ def test_fsim_phase_corrections(
),
)
def test_phase_corrected_fsim_operations_with_phase_exponent(
theta: float, zeta: float, chi: float, gamma: float, phi: float
theta: cirq.value.TParamVal, zeta: float, chi: float, gamma: float, phi: cirq.value.TParamVal
) -> None:
a, b = cirq.LineQubit.range(2)

Expand Down
2 changes: 1 addition & 1 deletion cirq-google/cirq_google/common_serializers.py
Expand Up @@ -513,7 +513,7 @@ def _add_phase_match(op: cirq.Operation, proto: v2.program_pb2.Operation):
# FSim serializer
# Only allows iswap, sqrt_iswap and their inverses, iswap, CZ, identity, and sycamore
# Note that not all combinations may not be available on all processors
def _can_serialize_limited_fsim(theta: float, phi: float):
def _can_serialize_limited_fsim(theta: cirq.value.TParamVal, phi: cirq.value.TParamVal):
# Symbols for LIMITED_FSIM are allowed, but may fail server-side
# if an incorrect run context is specified
if _near_mod_2pi(phi, 0) or isinstance(phi, sympy.Symbol):
Expand Down
Expand Up @@ -256,7 +256,7 @@ def decompose_phased_iswap_into_syc(


def decompose_phased_iswap_into_syc_precomputed(
theta: float, a: cirq.Qid, b: cirq.Qid
theta: cirq.value.TParamVal, a: cirq.Qid, b: cirq.Qid
) -> cirq.OP_TREE:
"""Decompose PhasedISwap into sycamore gates using precomputed coefficients.

Expand Down Expand Up @@ -466,7 +466,7 @@ def _phased_x_z_ops(mat: np.ndarray, q: cirq.Qid) -> Iterator[cirq.Operation]:
yield gate(q)


def rzz(theta: float, q0: cirq.Qid, q1: cirq.Qid) -> cirq.OP_TREE:
def rzz(theta: cirq.value.TParamVal, q0: cirq.Qid, q1: cirq.Qid) -> cirq.OP_TREE:
"""Generate exp(-1j * theta * zz) from Sycamore gates.

Args:
Expand Down Expand Up @@ -495,7 +495,7 @@ def rzz(theta: float, q0: cirq.Qid, q1: cirq.Qid) -> cirq.OP_TREE:
yield create_corrected_circuit(target_unitary, program, q0, q1)


def cphase(theta: float, q0: cirq.Qid, q1: cirq.Qid) -> cirq.OP_TREE:
def cphase(theta: cirq.value.TParamVal, q0: cirq.Qid, q1: cirq.Qid) -> cirq.OP_TREE:
"""
Implement a cphase using the Ising gate generated from 2 Sycamore gates

Expand All @@ -515,7 +515,7 @@ def cphase(theta: float, q0: cirq.Qid, q1: cirq.Qid) -> cirq.OP_TREE:
yield cirq.rz(theta / 2).on(q1)


def swap_rzz(theta: float, q0: cirq.Qid, q1: cirq.Qid) -> cirq.OP_TREE:
def swap_rzz(theta: cirq.value.TParamVal, q0: cirq.Qid, q1: cirq.Qid) -> cirq.OP_TREE:
"""
An implementation of SWAP * EXP(1j theta ZZ) using three sycamore gates.

Expand Down