Add Carry and Sum Operators #1169

DSGuala · 2021-03-25T22:14:42Z

Context:
Carry and sum operators are used to build quantum adders. These can be useful in various applications e.g. Shor's algorithm.

Description of the Change:
Added the basic QubitCarry and QubitSum operators to qubit.py

Benefits:
Will enable basic summation operations.

Possible Drawbacks:
The user still needs to organize the carry and sum operators to perform multi-bit additions.
There may be more optimal summation methods depending on the application (e.g. by using the quantum fourier transform)

codecov · 2021-03-25T22:33:31Z

Codecov Report

Merging #1169 (a902d99) into master (cb4d2cc) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1169   +/-   ##
=======================================
  Coverage   98.13%   98.13%           
=======================================
  Files         145      145           
  Lines       10984    11010   +26     
=======================================
+ Hits        10779    10805   +26     
  Misses        205      205

Impacted Files	Coverage Δ
pennylane/devices/default_mixed.py	`100.00% <ø> (ø)`
pennylane/devices/default_qubit.py	`100.00% <ø> (ø)`
pennylane/ops/qubit.py	`98.34% <100.00%> (+0.04%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb4d2cc...a902d99. Read the comment docs.

trbromley

Thanks @DSGuala, this PR is progressing nicely!

.github/CHANGELOG.md

pennylane/ops/qubit.py

trbromley · 2021-03-26T12:05:59Z

pennylane/ops/qubit.py

+    * Gradient recipe: TODO
+
+    Args:
+        wires (Union[Wires or int]): the wires the operation acts on


There are two ways we could think about this:

Either we just let the user pass in wires (must be 4 of them)

We have 4 separate arguments, e.g., input carry, output carry, input A, input B

I'm in favour of the first option, but as long as we make it clear which wire does what. This is similar to CNOT, where we have a single wires argument.

However, in going for the first option, we'll have to be more careful when we make an adder between two registers, since this Carry operation will work on a combination of those registers.

I also prefer the first option. I think it should be okay as long as we are clear about which wires do what.

pennylane/ops/qubit.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

Update default.mixed Add expand()

.github/CHANGELOG.md

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

Updated docstrings

trbromley

Thanks @DSGuala! I have requested some changes.

pennylane/ops/qubit.py

trbromley · 2021-04-09T21:02:36Z

doc/_static/ops/Carry.svg

@@ -0,0 +1,451 @@
+<?xml version="1.0" encoding="UTF-8"?>


On second thoughts, maybe doc/_static/templates/carry/ and doc/_static/templates/sum might be better spots for this content.

Although they are ops, they are more familiar conceptually as a template 👍

Question: if carry is thought more as a template than an op, should it be in the templates module? In addition, would we add it to the templates quickstart gallery?

Going even further, where do we draw the line between an op and a template? Are ops what we think of when we imagine low-level device gate sets (X, Y, RX, CNOT, CSWAP), whereas a template is a larger operation designed with a specific purpose in mind?

🤔 Thinking over my last comment:

Operations are general purpose unitaries, often relating to the underlying device architecture, that must be combined to build up problem-specific unitaries.

Templates are larger unitaries designed for specific algorithms or applications

Is this the distinction we want to draw?

I think this is important to consider, as it determines whether the source code lives in pennylane/templates or pennylane/ops/qubit.py (which is getting very crowded!)

@mariaschuld might also have some thoughts here.

This is a very good question 🤔 and the line can indeed be blurred.

Could we define a template to be a unitary where only the expand() method is known? At least for Carry and Sum, we know the matrix representation too.

But yeah, it's not fully clear where the line should be. The original motivation to put these as ops was:

We'll be using them as primitives within something that is definitely a template: the Adder circuit

Similar precedents are already ops, e.g., CRot has a matrix and decomposition, but is unlikely to be used directly on hardware (?)

Doesn't seem to fit what I expect from a template, e.g., is not "scalable" (i.e., can have variable depth/width) and doesn't have input parameters.

Could we define a template to be a unitary where only the expand() method is known? At least for Carry and Sum, we know the matrix representation too.

I'm not sure this makes sense either - there is nothing stopping a device from declaring that it supports an Op directly, causing the expand to be avoided. And, the matrix property is just for convenience, most devices ignore it.

I think the main difference will essentially come down to a conceptual difference, and will likely be flexible.

trbromley · 2021-04-09T21:04:45Z

doc/_static/ops/Carry.svg

@@ -0,0 +1,451 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="531.153pt" height="66.041pt" viewBox="0 0 531.153 66.041" version="1.1">


Are we sure about the bottom-right? I don't have a reason to think it's wrong, but just want to double check.

Ohhh so that's the carried value?

The bottom right comes from this paper and is indeed the carried value. Perhaps there is a clearer way to write it. Just |C> does not seem clear enough

trbromley · 2021-04-09T21:06:34Z

doc/_static/ops/Carry.svg

@@ -0,0 +1,451 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="531.153pt" height="66.041pt" viewBox="0 0 531.153 66.041" version="1.1">
+<defs>


For me, just the RHS one is sufficient

Perfect, will change this.

pennylane/ops/qubit.py

pennylane/devices/default_qubit.py

tests/ops/test_qubit_ops.py

trbromley · 2021-04-09T21:56:30Z

tests/ops/test_qubit_ops.py

+            ([0, 1, 2, 3], "0000", "0000"),
+            ([0, 1, 2, 3], "0010", "0010"),
+            ([0, 1, 2, 3], "0100", "0110"),
+            ([0, 1, 2, 3], "0110", "0101"),
+            ([0, 1, 2, 3], "1010", "1011"),


I like these! Ideally it would be good to cover all 16 eventualities, and confirm by hand that the second bitstring is indeed the carry.

Going over all 16 eventualities + changing to the tape mode with and without expansion should be a comprehensive enough set of tests (and same for Sum)

I like these! Ideally it would be good to cover all 16 eventualities, and confirm by hand that the second bitstring is indeed the carry.

Also that the sum is the correct one, right?

trbromley · 2021-04-09T21:56:40Z

tests/ops/test_qubit_ops.py

+            ([3, 1, 2, 0], "0110", "1100"),
+            ([3, 2, 0, 1], "1010", "0110"),


Nice to permute!

trbromley · 2021-04-09T21:59:39Z

tests/ops/test_qubit_ops.py

+            ([3, 2, 0, 1], "1010", "0110"),
+        ],
+    )
+    def test_carry(self, wires, input_string, output_string):


It would be good to test the expand() method. To do this, I suggest here having a parametrize over expand = True/False.

Then, below, instead of using

@qml.qnode(dev) def circuit(): ... return qml.probs()

You can do

with qml.tape.QuantumTape() as tape: ... qml.probs()

This is using the lower level tape functionality, which gives us control over expansion.

To execute, we'd then do

dev.execute(tape)

So, when it comes to expanding, before executing we'd do tape = tape.expand(), which will then allow us to test if the expansion is correct.

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

ixfoduap

Looks good so far!

My main comments are regarding docstrings, which I believe could be improved for further clarity

.github/CHANGELOG.md

ixfoduap · 2021-04-12T13:23:42Z

pennylane/ops/qubit.py

+
+class Carry(Operation):
+    r"""Carry(wires)
+    Apply the ``Carry`` operation to the input wires.


I'm left confused after reading the docstring. Can we provide a short intuitive explanation of what this operation does? Maybe with a simple but explicit example?

Is it correct to say that this operation performs addition of two bit values and stores the value that is carried over in a separate qubit? So for example it would map |0>|1>|1>|0>-->|1>|1>|0>|1>? I'm still confused about the role of the fourth qubit... 🤔

Yes! I will be improving the docstrings. The carry operation would map |0>|1>|1>|0> --> |0>|1>|0>|1>. The fourth qubit takes the carried value. I think the confusion here comes from the fact that Carry is a primitive operation for an Adder template. The fourth qubit actually takes the carry value of the first three qubits.

pennylane/ops/qubit.py

ixfoduap · 2021-04-12T17:46:32Z

pennylane/ops/qubit.py

+        wires (Union[Wires or int]): the wires the operation acts on
+
+    **Example**
+    TODO


Should add extra text explaining that qubit 3 is the carry qubit, otherwise it may be confusing why we apply Sum to four qubits only to sum three of them

ixfoduap · 2021-04-12T17:49:18Z

pennylane/ops/qubit.py

+
+class Sum(Operation):
+    r"""Sum()
+    Apply a ``Sum`` operation on the input wires.


This is not really accurate, is it? The operation is only summing up two of the wires, not all of them, right?

Hopefully this is clearer in the new version. For sum, it is indeed the modulo 2 sum of all input wires

ixfoduap · 2021-04-12T17:50:31Z

pennylane/ops/qubit.py

+        wires (Union[Wires or int]): the wires the operation acts on
+
+    **Example**
+    TODO


Actually, the operation only acts on three wires, right? So this should be wires=[0,1,2] and explain that one of the wires is a carry qubit

ixfoduap · 2021-04-12T17:53:12Z

pennylane/ops/qubit.py

+        wires (Union[Wires or int]): the wires the operation acts on
+
+    **Example**
+    TODO


Finally, these docstrings could also benefit from the example actually performing a specific sum, e.g. 1+1 = 0. And it would be good to explain everything is done modulo 2

ixfoduap · 2021-04-12T18:07:51Z

tests/ops/test_qubit_ops.py

+            ([0, 1, 2, 3], "0000", "0000"),
+            ([0, 1, 2, 3], "0010", "0010"),
+            ([0, 1, 2, 3], "0100", "0110"),
+            ([0, 1, 2, 3], "0110", "0101"),
+            ([0, 1, 2, 3], "1010", "1011"),


I like these! Ideally it would be good to cover all 16 eventualities, and confirm by hand that the second bitstring is indeed the carry.

Also that the sum is the correct one, right?

* Update imgs * Add to changelog * Add to docs * Update docs * Update expand * Add to docs * Update example * Typo * Add subtlety * Add * Update docstring * Update docs * Respond to suggestion

pennylane/ops/qubit.py

Co-authored-by: Chase Roberts <chase@xanadu.ai>

trbromley

Thanks @DSGuala!

trbromley

Thanks @DSGuala! Ready to merge - would be good to do one quick last check of the docs to see if everything is still rendering nicely.

chaserileyroberts · 2021-04-20T18:17:54Z

tests/ops/test_qubit_ops.py

@@ -401,7 +401,13 @@ def test_adjoint_unitaries(self, op, tol):
        np.testing.assert_allclose(res2, np.eye(2 ** len(op.wires)), atol=tol)
        assert op.wires == op_d.wires


Can you add QubitSum to this test aswell?

chaserileyroberts

Just add the test I mentioned earlier

Initial commit: basic Carry and Sum operations

268c040

DSGuala added the WIP 🚧 Work-in-progress label Mar 25, 2021

DSGuala requested review from ixfoduap and trbromley March 25, 2021 22:14

Changelog update

dee1513

trbromley reviewed Mar 26, 2021

View reviewed changes

DSGuala and others added 8 commits March 26, 2021 08:06

Update CHANGELOG

7bc1781

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

Update changelog example

07cb908

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

Update changelog example: standard return

1df5a03

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

Update changelog example: clarify input bitstring

1da5085

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

Update pennylane/ops/qubit.py: wire requirements comment

bfcdeee

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

Update changelog example: determining output

458f3fe

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

Update operations.rst

61b2d5e

Update default.mixed Add expand()

Merge branch 'master' into carry_sum

16c475c

trbromley reviewed Apr 5, 2021

View reviewed changes

.github/CHANGELOG.md Outdated Show resolved Hide resolved

.github/CHANGELOG.md Outdated Show resolved Hide resolved

.github/CHANGELOG.md Outdated Show resolved Hide resolved

DSGuala and others added 8 commits April 5, 2021 11:07

Update unresolved merge conflicts

ab309f5

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

Update unresolved merge conflicts

ab7c63e

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

Update unresolved merge conflicts

5774f4a

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

add sum diagram, update qubit.py, update carry-sum tests

ae35c76

updated docs

1ddf964

update docs

cfbb77b

update doc .svg diagrams

1dd61e3

Modified doc diagrams

71320eb

Updated docstrings

trbromley requested changes Apr 9, 2021

View reviewed changes

DSGuala and others added 4 commits April 9, 2021 17:14

Update .github/CHANGELOG.md

c791332

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

Update pennylane/ops/qubit.py

47de9ff

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

Update pennylane/ops/qubit.py

1131064

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

Update pennylane/ops/qubit.py

e7e6d99

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

ixfoduap reviewed Apr 12, 2021

View reviewed changes

DSGuala and others added 3 commits April 15, 2021 10:11

black -l 100

12d036a

Update diagrams, format Changelog

9a9aa9c

Update to docs for CarrySum (#1219)

f529068

* Update imgs * Add to changelog * Add to docs * Update docs * Update expand * Add to docs * Update example * Typo * Add subtlety * Add * Update docstring * Update docs * Respond to suggestion

chaserileyroberts reviewed Apr 15, 2021

View reviewed changes

pennylane/ops/qubit.py Show resolved Hide resolved

chaserileyroberts suggested changes Apr 15, 2021

View reviewed changes

pennylane/ops/qubit.py Outdated Show resolved Hide resolved

trbromley and others added 6 commits April 15, 2021 13:25

Update pennylane/ops/qubit.py

566f4fc

Co-authored-by: Chase Roberts <chase@xanadu.ai>

Fix adjoint

cb761c3

Add test

6c3107d

Use _mat

c5d9e2f

Check for decomposition

ded2c5b

Adjust size

a2969ec

trbromley approved these changes Apr 15, 2021

View reviewed changes

trbromley requested a review from chaserileyroberts April 15, 2021 20:09

ixfoduap approved these changes Apr 15, 2021

View reviewed changes

trbromley added do not merge ⚠️ Do not merge the pull request until this label is removed merge-ready ✔️ All tests pass and the PR is ready to be merged. labels Apr 15, 2021

josh146 removed the do not merge ⚠️ Do not merge the pull request until this label is removed label Apr 20, 2021

josh146 and others added 6 commits April 20, 2021 19:32

Merge branch 'master' into carry_sum

6a2d342

Merge branch 'master' into carry_sum

ab9369c

Fix changelog

267f617

Fix changelog

844e434

Fix changelog

c41f4e3

Simplify test

22eb64d

trbromley approved these changes Apr 20, 2021

View reviewed changes

chaserileyroberts reviewed Apr 20, 2021

View reviewed changes

chaserileyroberts approved these changes Apr 20, 2021

View reviewed changes

DSGuala added 2 commits April 20, 2021 13:30

Add QubitSum to test_adjoint_unitaries

234e36c

black -l 100

a902d99

DSGuala merged commit 4c6722d into master Apr 20, 2021

DSGuala deleted the carry_sum branch April 20, 2021 19:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Carry and Sum Operators #1169

Add Carry and Sum Operators #1169

DSGuala commented Mar 25, 2021 •

edited

Loading

codecov bot commented Mar 25, 2021 •

edited

Loading

trbromley left a comment

trbromley Mar 26, 2021

DSGuala Mar 31, 2021

trbromley left a comment

trbromley Apr 9, 2021

josh146 Apr 10, 2021

josh146 Apr 10, 2021

trbromley Apr 12, 2021

josh146 Apr 12, 2021

trbromley Apr 9, 2021

trbromley Apr 9, 2021

DSGuala Apr 12, 2021 •

edited

Loading

trbromley Apr 9, 2021

DSGuala Apr 12, 2021

trbromley Apr 9, 2021

trbromley Apr 9, 2021

ixfoduap Apr 12, 2021

trbromley Apr 9, 2021

trbromley Apr 9, 2021

ixfoduap left a comment

ixfoduap Apr 12, 2021

ixfoduap Apr 12, 2021

DSGuala Apr 12, 2021

ixfoduap Apr 12, 2021

ixfoduap Apr 12, 2021

DSGuala Apr 13, 2021

ixfoduap Apr 12, 2021

ixfoduap Apr 12, 2021

ixfoduap Apr 12, 2021

trbromley left a comment

trbromley left a comment

chaserileyroberts Apr 20, 2021

chaserileyroberts left a comment

		@@ -0,0 +1,451 @@
		<?xml version="1.0" encoding="UTF-8"?>
		<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="531.153pt" height="66.041pt" viewBox="0 0 531.153 66.041" version="1.1">

		([3, 1, 2, 0], "0110", "1100"),
		([3, 2, 0, 1], "1010", "0110"),

		@@ -401,7 +401,13 @@ def test_adjoint_unitaries(self, op, tol):
		np.testing.assert_allclose(res2, np.eye(2 ** len(op.wires)), atol=tol)
		assert op.wires == op_d.wires

Add Carry and Sum Operators #1169

Add Carry and Sum Operators #1169

Conversation

DSGuala commented Mar 25, 2021 • edited Loading

codecov bot commented Mar 25, 2021 • edited Loading

Codecov Report

trbromley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trbromley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DSGuala Apr 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ixfoduap left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trbromley left a comment

Choose a reason for hiding this comment

trbromley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chaserileyroberts left a comment

Choose a reason for hiding this comment

DSGuala commented Mar 25, 2021 •

edited

Loading

codecov bot commented Mar 25, 2021 •

edited

Loading

DSGuala Apr 12, 2021 •

edited

Loading