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

Added a custom gate application for Toffoli in default.qubit #1249

Merged
merged 26 commits into from
May 3, 2021
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
f220751
first draft implementation of _apply_toffoli
ryanhill1 Apr 21, 2021
01ab68b
second draft _apply_toffoli
ryanhill1 Apr 22, 2021
3c843aa
testing first pass with output
ryanhill1 Apr 22, 2021
5dbd7f9
_apply_toffoli is working with forward test case
ryanhill1 Apr 22, 2021
bf12f0d
passing all test cases
ryanhill1 Apr 22, 2021
1b88829
deleted comments, working for full pytest. still need documentation
ryanhill1 Apr 22, 2021
be46b9d
documentation done
ryanhill1 Apr 22, 2021
e2af541
fixed typo
ryanhill1 Apr 22, 2021
e78225a
Merge branch 'master' into toffoli
ryanhill1 Apr 22, 2021
208babb
Update CHANGELOG.md
ryanhill1 Apr 22, 2021
ee6e96d
Merge branch 'master' into toffoli
ryanhill1 Apr 22, 2021
d9daa23
replaced reshaping with stacking
ryanhill1 Apr 26, 2021
cf45a76
converting to 4 qubit tests
ryanhill1 Apr 27, 2021
fdf59b2
Merge remote-tracking branch 'origin/toffoli' into toffoli
ryanhill1 Apr 27, 2021
798acad
Merge branch 'master' into toffoli
antalszava Apr 27, 2021
4244ee1
still writing test cases and working on implementation, not ready for…
ryanhill1 Apr 27, 2021
066dc66
Merge remote-tracking branch 'origin/toffoli' into toffoli
ryanhill1 Apr 27, 2021
401ddd1
Merge branch 'master' into toffoli
ryanhill1 Apr 29, 2021
e487604
passing 21/24 4-qubit 3-permutations
ryanhill1 Apr 29, 2021
835f5c3
passing all test cases, documented & ran black
ryanhill1 Apr 30, 2021
81699ef
ran black on test_default_qubit.py
ryanhill1 Apr 30, 2021
b870e6b
wire ordering typo
ryanhill1 Apr 30, 2021
7638d1c
Merge branch 'master' into toffoli
antalszava Apr 30, 2021
501c847
Merge branch 'master' into toffoli
ryanhill1 Apr 30, 2021
da281ab
Merge branch 'master' into toffoli
antalszava May 3, 2021
0cd56a0
added `cntrl_min = cntrl_max ^ 1` for readability
ryanhill1 May 3, 2021
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: 4 additions & 1 deletion .github/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@

<h3>Improvements</h3>

* Added custom gate application for Toffoli in `default.qubit`.
[(#1249)](https://github.com/PennyLaneAI/pennylane/pull/1249)

* The device test suite now provides test cases for checking gates by comparing
expectation values.
[(#1212)](https://github.com/PennyLaneAI/pennylane/pull/1212)
Expand Down Expand Up @@ -65,7 +68,7 @@

This release contains contributions from (in alphabetical order):

Thomas Bromley, Diego Guala, Anthony Hayes, Josh Izaac, Antal Száva
Thomas Bromley, Diego Guala, Anthony Hayes, Ryan Hill, Josh Izaac, Antal Száva

# Release 0.15.0 (current release)

Expand Down
38 changes: 38 additions & 0 deletions pennylane/devices/default_qubit.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ def __init__(self, wires, *, shots=None, cache=0, analytic=None):
"CNOT": self._apply_cnot,
"SWAP": self._apply_swap,
"CZ": self._apply_cz,
"Toffoli": self._apply_toffoli,
}

def map_wires(self, wires):
Expand Down Expand Up @@ -336,6 +337,43 @@ def _apply_cnot(self, state, axes, **kwargs):
state_x = self._apply_x(state[sl_1], axes=target_axes)
return self._stack([state[sl_0], state_x], axis=axes[0])

def _apply_toffoli(self, state, axes, **kwargs):
"""Applies a Toffoli gate by slicing along the axis of the greater control qubit, slicing
each of the resulting sub-arrays along the axis of the smaller control qubit, and then applying
an X transformation along the axis of the target qubit of the fourth sub-sub-array.

By performing two consecutive slices in this way, we are able to select all of the amplitudes with
a corresponding :math:`|11\rangle` for the two control qubits. This means we then just need to apply
a :class:`~.PauliX` (NOT) gate to the result.

Args:
state (array[complex]): input state
axes (List[int]): target axes to apply transformation

Returns:
array[complex]: output state
"""
cntrl_max = np.argmax(axes[:2])
sl_a0 = _get_slice(0, axes[cntrl_max], self.num_wires)
sl_a1 = _get_slice(1, axes[cntrl_max], self.num_wires)
sl_b0 = _get_slice(0, axes[cntrl_max ^ 1], self.num_wires - 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Like the use of ^! very clever way of doing this 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe worth adding cntrl_min = cntrl_max ^ 1 after line cntrl_max and using cntrl_min for readability

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just added that change, it does look better!

sl_b1 = _get_slice(1, axes[cntrl_max ^ 1], self.num_wires - 1)

# If both controls are smaller than the target, shift the target axis down by two. If one
# control is greater and one control is smaller than the target, shift the target axis
# down by one. If both controls are greater than the target, leave the target axis as-is.
if axes[cntrl_max ^ 1] > axes[2]:
target_axes = [axes[2]]
elif axes[cntrl_max] > axes[2]:
target_axes = [axes[2] - 1]
else:
target_axes = [axes[2] - 2]

# state[sl_a1][sl_b1] gives us all of the amplitudes with a |11> for the two control qubits.
state_x = self._apply_x(state[sl_a1][sl_b1], axes=target_axes)
state_stacked_a1 = self._stack([state[sl_a1][sl_b0], state_x], axis=axes[cntrl_max ^ 1])
return self._stack([state[sl_a0], state_stacked_a1], axis=axes[cntrl_max])

def _apply_swap(self, state, axes, **kwargs):
"""Applies a SWAP gate by performing a partial transposition along the specified axes.

Expand Down
46 changes: 41 additions & 5 deletions tests/devices/test_default_qubit.py
Original file line number Diff line number Diff line change
Expand Up @@ -1909,8 +1909,8 @@ class TestApplyOps:
"""Tests for special methods listed in _apply_ops that use array manipulation tricks to apply
gates in DefaultQubit."""

state = np.arange(2 ** 3).reshape((2, 2, 2))
dev = qml.device("default.qubit", wires=3)
state = np.arange(2 ** 4).reshape((2, 2, 2, 2))
Copy link
Contributor

Choose a reason for hiding this comment

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

How come we had to go for a device with 4 qubits?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it to a device with 4 qubits so we can test wire orderings in which the two controlled bits and the target bit are not all adjacent to one another. This helps ensure that _apply_toffoli works for any number of wires, not just three.

dev = qml.device("default.qubit", wires=4)
single_qubit_ops = [
(qml.PauliX, dev._apply_x),
(qml.PauliY, dev._apply_y),
Expand All @@ -1925,14 +1925,17 @@ class TestApplyOps:
(qml.SWAP, dev._apply_swap),
(qml.CZ, dev._apply_cz),
]
three_qubit_ops = [
(qml.Toffoli, dev._apply_toffoli),
]

@pytest.mark.parametrize("op, method", single_qubit_ops)
def test_apply_single_qubit_op(self, op, method, inverse):
"""Test if the application of single qubit operations is correct."""
state_out = method(self.state, axes=[1], inverse=inverse)
op = op(wires=[1])
matrix = op.inv().matrix if inverse else op.matrix
state_out_einsum = np.einsum("ab,ibk->iak", matrix, self.state)
state_out_einsum = np.einsum("ab,ibjk->iajk", matrix, self.state)
assert np.allclose(state_out, state_out_einsum)

@pytest.mark.parametrize("op, method", two_qubit_ops)
Expand All @@ -1942,7 +1945,7 @@ def test_apply_two_qubit_op(self, op, method, inverse):
op = op(wires=[0, 1])
matrix = op.inv().matrix if inverse else op.matrix
matrix = matrix.reshape((2, 2, 2, 2))
state_out_einsum = np.einsum("abcd,cdk->abk", matrix, self.state)
state_out_einsum = np.einsum("abcd,cdjk->abjk", matrix, self.state)
assert np.allclose(state_out, state_out_einsum)

@pytest.mark.parametrize("op, method", two_qubit_ops)
Expand All @@ -1953,7 +1956,40 @@ def test_apply_two_qubit_op_reverse(self, op, method, inverse):
op = op(wires=[2, 1])
matrix = op.inv().matrix if inverse else op.matrix
matrix = matrix.reshape((2, 2, 2, 2))
state_out_einsum = np.einsum("abcd,idc->iba", matrix, self.state)
state_out_einsum = np.einsum("abcd,idck->ibak", matrix, self.state)
assert np.allclose(state_out, state_out_einsum)

@pytest.mark.parametrize("op, method", three_qubit_ops)
def test_apply_three_qubit_op_controls_smaller(self, op, method, inverse):
"""Test if the application of three qubit operations is correct when both control wires are
smaller than the target wire."""
state_out = method(self.state, axes=[0, 2, 3])
op = op(wires=[0, 2, 3])
Comment on lines +1966 to +1967
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be less error-prone upon future modification and updates if it's explicit that these two variables are the same thing. Same for the other tests.

Also, we could pull out that variable and parameterize the test over wire order. It would make the code more concise and easier to maintain.

Suggested change
state_out = method(self.state, axes=[0, 2, 3])
op = op(wires=[0, 2, 3])
wires_order = (0,2,3)
state_out = method(self.state, axes=wires_order)
op = op(wires=wires_order)

Copy link
Contributor Author

@ryanhill1 ryanhill1 May 3, 2021

Choose a reason for hiding this comment

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

I've started pulling out the wires variables and parameterizing the tests over the wire order, but I'm having trouble elegantly replacing the einsum functions with tensordot functions. tensordot does not provide a way of reordering the noncontracted dimensions, so I'm instead needing to slice along the control axis, apply tensordot to each slice, and then reassemble the state. But this seems much too similar to the actual implementation to be a fair test. For example, in test_apply_single_qubit_op, if wires_order = [0], then our test state can simply be constructed using state_out_tensordot = np.tensordot(matrix, self.state). But if wires_order = [1], we need to do

result0 = np.tensordot(matrix, self.state[0])
result1 = np.tensordot(matrix, self.state[1])
state_out_tensordot = np.stack([result0, result1])

which gets even more involved for higher dimensions, and therefore is hard to generalize without basically copying the _apply_toffoli implementation. I've tried specifying different axes along which to apply tensordot, but the problem again is the ordering of dimensions. tensordot is fine for einsum operations such as ab,bijk->aijk, because the output is the default ordering. But it seems operations such as ab,ibjk->iajk are not reproducible in tensordot without further specification. And so the general case i.e. making one specification that works for any wires_order, will require if/else statements inside the testing function, which is not ideal. Do you have any ideas?

Edit: I might have just found a workound using np.kron and np.eye to expand the dimensional representation of matrix according to wires_order.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry this didn't work out. The idea was to make the tests simpler, but this sounds like it just makes the tests more complicated.

It was more of a suggestion than a need, so I'm going ahead with changing my review to "approve". Thanks for trying it out anyway 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I think that's right the call. The parameterized version of test_apply_single_qubit_op would have needed to look something like this:

num_wires = 4
state = np.arange(2 ** num_wires)

...

single_qubit_wires_order = [
    [1],
]

...

@pytest.mark.parametrize("op, method", single_qubit_ops)
@pytest.mark.parametrize("wires_order", single_qubit_wires_order)
def test_apply_single_qubit_op(self, op, method, wires_order, inverse):
    """Test if the application of single qubit operations is correct."""
    state_out = method(self.state.reshape([2] * self.num_wires), axes=wires_order, inverse=inverse)
    op = op(wires=wires_order)
    matrix0 = op.inv().matrix if inverse else op.matrix
    matrix1 = np.kron(np.eye(2 ** wires_order[0]), matrix0)
    matrix2 = np.kron(matrix1, np.eye(2 ** (self.num_wires - wires_order[0] - 1)))
    state_out_dot = np.dot(matrix2, self.state).reshape([2] * self.num_wires)
    assert np.allclose(state_out, state_out_dot)

matrix = op.inv().matrix if inverse else op.matrix
matrix = matrix.reshape((2, 2) * 3)
state_out_einsum = np.einsum("abcdef,dkef->akbc", matrix, self.state)
assert np.allclose(state_out, state_out_einsum)

@pytest.mark.parametrize("op, method", three_qubit_ops)
def test_apply_three_qubit_op_controls_greater(self, op, method, inverse):
"""Test if the application of three qubit operations is correct when both control wires are
greater than the target wire."""
state_out = method(self.state, axes=[2, 1, 0])
op = op(wires=[2, 1, 0])
matrix = op.inv().matrix if inverse else op.matrix
matrix = matrix.reshape((2, 2) * 3)
state_out_einsum = np.einsum("abcdef,fedk->cbak", matrix, self.state)
assert np.allclose(state_out, state_out_einsum)

@pytest.mark.parametrize("op, method", three_qubit_ops)
def test_apply_three_qubit_op_controls_split(self, op, method, inverse):
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for adding this test case!

"""Test if the application of three qubit operations is correct when one control wire is smaller
and one control wire is greater than the target wire."""
state_out = method(self.state, axes=[3, 1, 2])
op = op(wires=[3, 1, 2])
matrix = op.inv().matrix if inverse else op.matrix
matrix = matrix.reshape((2, 2) * 3)
state_out_einsum = np.einsum("abcdef,kdfe->kacb", matrix, self.state)
assert np.allclose(state_out, state_out_einsum)


Expand Down