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.

Sign up for GitHub

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

Jump to bottom

Make cirq.FREDKIN gate self-inverse #6135

Merged
merged 4 commits into from
Jun 11, 2023
Merged

Make cirq.FREDKIN gate self-inverse #6135

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d067135
Make FREDKIN gate self-inverse
tanujkhattar Jun 11, 2023
ca04ea6
Fix formatting
tanujkhattar Jun 11, 2023
9b41dea
Merge branch 'master' of https://github.com/quantumlib/cirq into fred…
tanujkhattar Jun 11, 2023
086679f
Simplify test to check CSWAP matrix is self inverse
tanujkhattar Jun 11, 2023
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
5 changes: 5 additions & 0 deletions cirq-core/cirq/ops/three_qubit_gates.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,11 @@ def _qasm_(self, args: 'cirq.QasmArgs', qubits: Tuple['cirq.Qid', ...]) -> Optio
def _value_equality_values_(self):
return ()

def __pow__(self, power):
if power == 1 or power == -1:
return self
return NotImplemented

def __str__(self) -> str:
return 'FREDKIN'

Expand Down
7 changes: 6 additions & 1 deletion cirq-core/cirq/ops/three_qubit_gates_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,11 @@ def test_unitary():
),
atol=1e-8,
)
np.testing.assert_allclose(
cirq.unitary(cirq.FREDKIN**-1),
cirq.linalg.map_eigenvalues(cirq.unitary(cirq.FREDKIN), lambda b: b**-1),
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 need the call to map_eigevalues:

np.testing.assert_allclose(cirq.unitary(cirq.FREDKIN**-1), cirq.unitary(cirq.FREDKIN), 1e-8)

Mathematical reason is that the eigenvalues of FREDKIN are all $\pm 1$, so $\lambda=\lambda^{-1}$. Pythonic reason is that AFAICT we should have equality cirq.FREDKIN**-1 == cirq.FREDKIN, before we even call cirq.unitary. Perhaps that is a better (or additional) thing to assert (though perhaps not inside test_unitary, not sure what a better suited test is). Any potential extra assert is up to you.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I agree we should add a assert cirq.FREDKIN ** -1 == cirq.FREDKIN. The reason I used this approach for the test here is to assert that the unitary matrix itself is self inverse, and not the object. Self inverse nature of the unitary matrix can be figured out using the math, but here the goal was to test that math.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The simplest way to do this is

u = cirq.unitary(cirq.FREDKIN)
np.testing.assert_allclose(u @ u, np.eye(8))

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Simplified the test for self-inverse as recommended. Also, note that assert cirq.FREDKIN ** -1 == cirq.FREDKIN is already covered in my original PR under the equality test eq.add_equality_group(cirq.CSWAP(a, b, c), cirq.FREDKIN(a, b, c), cirq.FREDKIN(a, b, c) ** -1). Merging now.

atol=1e-6
)

diagonal_angles = [2, 3, 5, 7, 11, 13, 17, 19]
assert cirq.has_unitary(cirq.ThreeQubitDiagonalGate(diagonal_angles))
Expand Down Expand Up @@ -156,7 +161,7 @@ def test_eq():
eq.add_equality_group(cirq.TOFFOLI(a, b, c), cirq.CCX(a, b, c))
eq.add_equality_group(cirq.TOFFOLI(a, c, b), cirq.TOFFOLI(c, a, b))
eq.add_equality_group(cirq.TOFFOLI(a, b, d))
eq.add_equality_group(cirq.CSWAP(a, b, c), cirq.FREDKIN(a, b, c))
eq.add_equality_group(cirq.CSWAP(a, b, c), cirq.FREDKIN(a, b, c), cirq.FREDKIN(a, b, c) ** -1)
eq.add_equality_group(cirq.CSWAP(b, a, c), cirq.CSWAP(b, c, a))


Expand Down