Skip to content

Bring canonicalization of FSimGate and PhasedFSimGate angles b/w [-pi, pi) #4576

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

Merged
merged 4 commits into from
Oct 16, 2021
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions cirq-core/cirq/ops/fsim_gate.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,20 +35,20 @@


def _canonicalize(value: Union[float, sympy.Basic]) -> Union[float, sympy.Basic]:
"""Assumes value is 2π-periodic and shifts it into [-π, π]."""
"""Assumes value is 2π-periodic and shifts it into [-π, π)."""
if protocols.is_parameterized(value):
return value
period = 2 * np.pi
return value - period * np.round(value / period)
return value - period * np.floor((value + np.pi) / period)


def _zero_mod_pi(param: Union[float, sympy.Basic]) -> bool:
"""Returns True iff param, assumed to be in [-pi, pi], is 0 (mod pi)."""
return param in (-np.pi, 0.0, np.pi, -sympy.pi, sympy.pi)
"""Returns True iff param, assumed to be in [-pi, pi), is 0 (mod pi)."""
return param in (0.0, -np.pi, -sympy.pi)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Optional: We could add an assert to verify that param is in [-pi, pi). Now that the result of these calculations will be used for equality checks, it's probably better to have the code crash and signal a bug than silently pretend equal things are different.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The equality checks are done by directly comparing the self.theta etc. angles, and these functions are also called only with the class angles, which are always canonicalized. I could add an assert to the _canonicalize method which ensures that the return value is between [-pi, pi), but the tests already ensure that the method works as expected so I don't think the assert is needed.



def _half_pi_mod_pi(param: Union[float, sympy.Basic]) -> bool:
"""Returns True iff param, assumed to be in [-pi, pi], is pi/2 (mod pi)."""
"""Returns True iff param, assumed to be in [-pi, pi), is pi/2 (mod pi)."""
return param in (-np.pi / 2, np.pi / 2, -sympy.pi / 2, sympy.pi / 2)


Expand Down
47 changes: 26 additions & 21 deletions cirq-core/cirq/ops/fsim_gate_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ def test_fsim_eq():
eq.add_equality_group(cirq.FSimGate(0, 0))
eq.add_equality_group(cirq.FSimGate(1, 1))
eq.add_equality_group(cirq.FSimGate(1, 2).on(a, b), cirq.FSimGate(1, 2).on(b, a))
eq.add_equality_group(cirq.FSimGate(np.pi, np.pi), cirq.FSimGate(-np.pi, -np.pi))


def test_fsim_approx_eq():
Expand Down Expand Up @@ -73,26 +74,26 @@ def test_fsim_circuit():
cirq.testing.assert_has_diagram(
c,
"""
0: ───FSim(0.5π, π)───FSim(-π, 0.5π)───
│ │
1: ───FSim(0.5π, π)───FSim(-π, 0.5π)───
0: ───FSim(0.5π, -π)───FSim(-π, 0.5π)───
1: ───FSim(0.5π, -π)───FSim(-π, 0.5π)───
""",
)
cirq.testing.assert_has_diagram(
c,
"""
0: ---FSim(0.5pi, pi)---FSim(-pi, 0.5pi)---
| |
1: ---FSim(0.5pi, pi)---FSim(-pi, 0.5pi)---
0: ---FSim(0.5pi, -pi)---FSim(-pi, 0.5pi)---
| |
1: ---FSim(0.5pi, -pi)---FSim(-pi, 0.5pi)---
""",
use_unicode_characters=False,
)
cirq.testing.assert_has_diagram(
c,
"""
0: ---FSim(1.5707963267948966, pi)---FSim(-pi, 1.5707963267948966)---
| |
1: ---FSim(1.5707963267948966, pi)---FSim(-pi, 1.5707963267948966)---
0: ---FSim(1.5707963267948966, -pi)---FSim(-pi, 1.5707963267948966)---
| |
1: ---FSim(1.5707963267948966, -pi)---FSim(-pi, 1.5707963267948966)---
""",
use_unicode_characters=False,
precision=None,
Expand Down Expand Up @@ -379,6 +380,10 @@ def test_phased_fsim_eq():
eq.add_equality_group(cirq.PhasedFSimGate(1, s, 0, 0, 0))
eq.add_equality_group(cirq.PhasedFSimGate(r, 1, 0, 0, 0))
eq.add_equality_group(cirq.PhasedFSimGate(s, 1, 0, 0, 0))
eq.add_equality_group(
cirq.PhasedFSimGate(np.pi, np.pi, np.pi, np.pi, np.pi),
cirq.PhasedFSimGate(-np.pi, -np.pi, -np.pi, -np.pi, -np.pi),
)

# Regions of insensitivity to zeta and chi
eq.add_equality_group(
Expand All @@ -392,7 +397,7 @@ def test_phased_fsim_eq():
cirq.PhasedFSimGate(sympy.pi / 2, 0, 0, 4, 5), cirq.PhasedFSimGate(sympy.pi / 2, 2, 0, 4, 5)
)
eq.add_equality_group(
cirq.PhasedFSimGate(sympy.pi, 0, 0, 4, 5), cirq.PhasedFSimGate(sympy.pi, 0, 3, 4, 5)
cirq.PhasedFSimGate(-sympy.pi, 0, 0, 4, 5), cirq.PhasedFSimGate(-sympy.pi, 0, 3, 4, 5)
)

# Symmetries under qubit exchange
Expand All @@ -410,8 +415,8 @@ def test_phased_fsim_eq():
cirq.PhasedFSimGate(1, -np.pi, np.pi, 4, 5).on(b, a),
)
eq.add_equality_group(
cirq.PhasedFSimGate(1, sympy.pi, -sympy.pi, r, 5).on(a, b),
cirq.PhasedFSimGate(1, sympy.pi, -sympy.pi, r, 5).on(b, a),
cirq.PhasedFSimGate(1, -sympy.pi, -sympy.pi, r, 5).on(a, b),
cirq.PhasedFSimGate(1, -sympy.pi, -sympy.pi, r, 5).on(b, a),
)
eq.add_equality_group(cirq.PhasedFSimGate(sympy.pi / 3, 2, 0, 4, 5).on(a, b))
eq.add_equality_group(cirq.PhasedFSimGate(sympy.pi / 3, 2, 0, 4, 5).on(b, a))
Expand Down Expand Up @@ -479,27 +484,27 @@ def test_phased_fsim_circuit():
cirq.testing.assert_has_diagram(
c,
"""
0: ───PhFSim(0.5π, π, 0.5π, 0, -0.25π)───PhFSim(-π, 0.5π, 0.1π, 0.2π, 0.3π)───
│ │
1: ───PhFSim(0.5π, π, 0.5π, 0, -0.25π)───PhFSim(-π, 0.5π, 0.1π, 0.2π, 0.3π)───
0: ───PhFSim(0.5π, -π, 0.5π, 0, -0.25π)───PhFSim(-π, 0.5π, 0.1π, 0.2π, 0.3π)───
1: ───PhFSim(0.5π, -π, 0.5π, 0, -0.25π)───PhFSim(-π, 0.5π, 0.1π, 0.2π, 0.3π)───
""",
)
# pylint: disable=line-too-long
cirq.testing.assert_has_diagram(
c,
"""
0: ---PhFSim(0.5pi, pi, 0.5pi, 0, -0.25pi)---PhFSim(-pi, 0.5pi, 0.1pi, 0.2pi, 0.3pi)---
| |
1: ---PhFSim(0.5pi, pi, 0.5pi, 0, -0.25pi)---PhFSim(-pi, 0.5pi, 0.1pi, 0.2pi, 0.3pi)---
0: ---PhFSim(0.5pi, -pi, 0.5pi, 0, -0.25pi)---PhFSim(-pi, 0.5pi, 0.1pi, 0.2pi, 0.3pi)---
| |
1: ---PhFSim(0.5pi, -pi, 0.5pi, 0, -0.25pi)---PhFSim(-pi, 0.5pi, 0.1pi, 0.2pi, 0.3pi)---
""",
use_unicode_characters=False,
)
cirq.testing.assert_has_diagram(
c,
"""
0: ---PhFSim(1.5707963267948966, pi, 1.5707963267948966, 0, -0.7853981633974483)---PhFSim(-pi, 1.5707963267948966, 0.3141592653589793, 0.6283185307179586, 0.9424777960769379)---
| |
1: ---PhFSim(1.5707963267948966, pi, 1.5707963267948966, 0, -0.7853981633974483)---PhFSim(-pi, 1.5707963267948966, 0.3141592653589793, 0.6283185307179586, 0.9424777960769379)---
0: ---PhFSim(1.5707963267948966, -pi, 1.5707963267948966, 0, -0.7853981633974483)---PhFSim(-pi, 1.5707963267948966, 0.3141592653589793, 0.6283185307179586, 0.9424777960769379)---
| |
1: ---PhFSim(1.5707963267948966, -pi, 1.5707963267948966, 0, -0.7853981633974483)---PhFSim(-pi, 1.5707963267948966, 0.3141592653589793, 0.6283185307179586, 0.9424777960769379)---
""",
use_unicode_characters=False,
precision=None,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,8 @@ def test_wait_gate_multi_qubit():
(cirq.CZ, 0, np.pi),
(cirq.CZ ** -1.0, 0, np.pi),
(cirq.FSimGate(theta=0, phi=0), 0, 0),
(cirq.FSimGate(theta=0, phi=np.pi), 0, np.pi),
(cirq.FSimGate(theta=0, phi=np.pi), 0, -np.pi),
(cirq.FSimGate(theta=0, phi=-np.pi), 0, -np.pi),
(cirq.FSimGate(theta=np.pi / 4, phi=0), np.pi / 4, 0),
(cirq.FSimGate(theta=7 * np.pi / 4, phi=0), -np.pi / 4, 0),
(cirq.FSimGate(theta=-np.pi / 4, phi=0), -np.pi / 4, 0),
Expand Down