Hardware-friendly decomposition #363

shreyapkumar · 2020-04-14T00:00:26Z

Context:
This PR attempts to remove needless complications in the code arising from the difference between how theorists define T matrices and the equivalent transformations that the chip implements.

Currently, the unitary is decomposed into Clements T matrices which are later converted to phases on the chip by rectangular_symmetric function. Decomposing the unitary directly into Mach Zehnder units would make the code simpler. It would also make it easier to modify the code and decompositions as planned in order to account for imperfect beamsplitters and also for power reduction, etc.

Description of the Change:

Adds MZ matrices to eventually replace T matrices to now represent the units on the chip.
Removes need for function rectangular_symmetric by modifying rectangular_phase_end_MZ to directly calculate internal and external phases on chip.
Tests very similar to the ones already present are added.

Possible Drawbacks:

Doesn't match the theoretical conventions in Clements and Xanadu modular decompositions.

shreyapkumar · 2020-04-14T00:03:37Z

Possible reviewers could be @lneuhaus , @nquesada and/or @ishdhand

codecov · 2020-04-14T00:37:17Z

Codecov Report

Merging #363 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #363   +/-   ##
=======================================
  Coverage   97.77%   97.77%           
=======================================
  Files          52       52           
  Lines        6435     6435           
=======================================
  Hits         6292     6292           
  Misses        143      143

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 4844687...4844687. Read the comment docs.

josh146

Looks great @shreyapkumar! I left some comments regarding code/documentation/tests, but will leave it to @nquesada and @ishdhand for code review on the decomposition logic itself 🙂

strawberryfields/decompositions.py

josh146 · 2020-04-14T03:22:21Z

strawberryfields/decompositions.py

+def BS_mat(theta):
+    r"""Beam splitter matrix as implemented in hardware
+    """
+    mat = np.array([[np.cos(theta), 1j*np.sin(theta)], [1j*np.sin(theta), np.cos(theta)]])


Just curious, off the top of my head, the default SF beamsplitter is the following?

np.array([[np.cos(theta), -np.exp(-1j*phi)*np.sin(theta)], [np.exp(1j*phi)*np.sin(theta), np.cos(theta)]])

So isn't this the same as the default SF beamsplitter, just with phi->pi/2? Although I might be misremembering the SF default.

strawberryfields/decompositions.py

josh146 · 2020-04-14T03:32:35Z

@shreyapkumar, by the way, CodeFactor runs some static analysis on the PR, and picked up the following issues. Running the Black formatter locally should automatically fix most of them,

black -l 100 strawberryfields/decompositions.py

and the remainder can be fixed by hand.

CodeFactor found multiple issues last seen at 2fa0b43:

Redefining name 'T' from outer scope (line 233)

strawberryfields\decompositions.py:499

No space allowed before :

if U[n, m] == 0 :
                ^

strawberryfields\decompositions.py:541

Trailing whitespace

strawberryfields\decompositions.py:493
strawberryfields\decompositions.py:654
strawberryfields\decompositions.py:661
strawberryfields\decompositions.py:492
strawberryfields\decompositions.py:548
strawberryfields\decompositions.py:500

No space allowed before :

if U[m, n] == 0 :
                ^

strawberryfields\decompositions.py:520

Co-Authored-By: Josh Izaac <josh146@gmail.com>

nquesada · 2020-04-17T00:19:11Z

strawberryfields/decompositions.py

@@ -400,6 +400,114 @@ def mach_zehnder(m, n, internal_phase, external_phase, nmax):
    return BS @ Rinternal @ BS @ Rexternal


+def mach_zehnder_inv(m, n, phi_int, phi_ext, nmax):
+    r"""The inverse of the mach_zehnder matrix."""
+    return np.transpose(np.conj(mach_zehnder(m, n, phi_int, phi_ext, nmax)))


Suggested change

return np.transpose(np.conj(mach_zehnder(m, n, phi_int, phi_ext, nmax)))

return mach_zehnder(m, n, phi_int, phi_ext, nmax).T.conj()

strawberryfields/decompositions.py

nquesada · 2020-04-17T00:22:08Z

strawberryfields/decompositions.py

+    localV = V
+    (nsize, _) = localV.shape
+
+    diffn = np.linalg.norm(V @ V.conj().T - np.identity(nsize))


Wonder if it would be good to compare closeness to the identity using np.allclose

I think the test here is a bit more strict than using np.allclose.
For example V = U + tol * np.identity(n)fails the above test, but passes the np.allclosefor the same tolerance.

That said, good to move this and other decompositions to np.allclose as that is easier to read and might be faster? Do you (or @josh146) have any idea about any potential downsides?

Good to use allclose if possible to keep things consistent (all the tests use the np.allclose(result, expected, atol=tol, rtol=0) pattern

nquesada · 2020-04-17T00:24:43Z

tests/frontend/test_decompositions.py

+        beamsplitters are multiplied together. Test passes if the product
+        matches the drawn unitary.
+        """
+        # TODO: this test currently uses the T and Ti functions used to compute


Is this meant to be fixed in this PR?

Oops, this was copied from other tests. I don't really know what this todo means, so removed this from the docstrings of test_random_unitary_MZ and test_identity_MZ

nquesada

Looks really great @shreyapkumar

shreyapkumar · 2020-04-20T00:00:19Z

Documentation and formatting changes are fixed now. Comments about this or anything else are welcome!

josh146

Looking good @shreyapkumar, my comments are all minor, and once addressed can be merged in 🙂

strawberryfields/decompositions.py

ishdhand

Looks great to me! Thanks to Josh for the many helpful comments, which I second. Other than those, no changes from my side.

Co-Authored-By: Josh Izaac <josh146@gmail.com>

…strawberryfields into HW-friendly-decomp

shreyapkumar · 2020-04-20T20:19:38Z

Thanks so much, @josh146 for the very helpful comments! I learnt a lot :)

shreyapkumar · 2020-04-20T20:34:22Z

tests/frontend/test_circuitspecs_X8.py

@@ -375,10 +375,10 @@ def test_no_unitary(self, tol):
            ops.MZgate(np.pi, 0) | (q[0], q[1])
            ops.MZgate(np.pi, 0) | (q[2], q[3])
            ops.MZgate(np.pi, np.pi) | (q[1], q[2])
-            ops.MZgate(np.pi, np.pi) | (q[0], q[1])
+            ops.MZgate(np.pi, 0) | (q[0], q[1])


Oops, looks like this file's changes came in from some merge from upstream. Not sure what to do about it!

Oh, that was me when I merged master into this branch! I noticed that this was the test that was failing

josh146

Thanks @shreyapkumar!

strawberryfields/decompositions.py

lneuhaus

Nice PR!

josh146 · 2020-04-30T14:04:44Z

@shreyapkumar, is this ready to be merged in?

shreyapkumar · 2020-04-30T14:18:39Z

Yes, nothing more to add from my side!

First commit for hardware friendly decomposition.

2fa0b43

josh146 requested review from lneuhaus, ishdhand and nquesada April 14, 2020 03:03

josh146 added frontend review-ready labels Apr 14, 2020

josh146 requested changes Apr 14, 2020

View reviewed changes

Merge remote-tracking branch 'upstream/master' into HW-friendly-decomp

e426eb0

josh146 mentioned this pull request Apr 16, 2020

Blackbird Program Compliation Bug #368

Closed

shreyapkumar and others added 7 commits April 16, 2020 17:34

formatting changes

794dbde

Black code formatting

45cd5af

sort codefactor issues

02c7a8b

BS docstring update

2377861

Co-Authored-By: Josh Izaac <josh146@gmail.com>

Merged MZ and mach_zehnder

425deaa

Removes rectangular_symmetric

206283a

Undoes unintentional PEP8 autoformat

b82e265

nquesada reviewed Apr 17, 2020

View reviewed changes

strawberryfields/decompositions.py Show resolved Hide resolved

nquesada reviewed Apr 17, 2020

View reviewed changes

shreyapkumar added 3 commits April 19, 2020 16:47

Changed tests to compare closeness to allclose

78ef47d

Updated docstrings

15c3e04

Removes trailing whitespaces

3af93be

Merge remote-tracking branch 'upstream/master' into HW-friendly-decomp

dfd6172

josh146 requested changes Apr 20, 2020

View reviewed changes

merge master

519e13a

ishdhand approved these changes Apr 20, 2020

View reviewed changes

shreyapkumar and others added 11 commits April 20, 2020 14:43

Update strawberryfields/decompositions.py

d7a330d

Co-Authored-By: Josh Izaac <josh146@gmail.com>

Update strawberryfields/decompositions.py

4d95f8b

Co-Authored-By: Josh Izaac <josh146@gmail.com>

Update strawberryfields/decompositions.py

9e1e3e5

Co-Authored-By: Josh Izaac <josh146@gmail.com>

Update strawberryfields/decompositions.py

13cea01

Co-Authored-By: Josh Izaac <josh146@gmail.com>

Update strawberryfields/decompositions.py

c0b7148

Co-Authored-By: Josh Izaac <josh146@gmail.com>

Update strawberryfields/decompositions.py

41785de

Co-Authored-By: Josh Izaac <josh146@gmail.com>

Update strawberryfields/decompositions.py

f09725a

Co-Authored-By: Josh Izaac <josh146@gmail.com>

Update strawberryfields/decompositions.py

a62020d

Co-Authored-By: Josh Izaac <josh146@gmail.com>

formatting changes

f149f2d

Merge branch 'HW-friendly-decomp' of https://github.com/shreyapkumar/…

58f507b

…strawberryfields into HW-friendly-decomp

Comments on different cases of decomposition

426ccb1

black formatting

98423dc

shreyapkumar commented Apr 20, 2020

View reviewed changes

josh146 approved these changes Apr 21, 2020

View reviewed changes

josh146 reviewed Apr 21, 2020

View reviewed changes

strawberryfields/decompositions.py Outdated Show resolved Hide resolved

Update strawberryfields/decompositions.py

fb749b4

lneuhaus approved these changes Apr 22, 2020

View reviewed changes

Update tests of rectangular decomposition to cover all the code

7544d5f

shreyapkumar added merge-ready and removed review-ready labels Apr 27, 2020

clean up tests

8916c1a

josh146 added 2 commits May 5, 2020 13:10

Merge remote-tracking branch 'upstream/master' into HW-friendly-decomp

fb4993d

updated changelog

4844687

josh146 merged commit 2de5a0f into XanaduAI:master May 5, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hardware-friendly decomposition #363

Hardware-friendly decomposition #363

shreyapkumar commented Apr 14, 2020

shreyapkumar commented Apr 14, 2020

codecov bot commented Apr 14, 2020 •

edited

josh146 left a comment

josh146 Apr 14, 2020

josh146 commented Apr 14, 2020 •

edited

nquesada Apr 17, 2020

nquesada Apr 17, 2020

shreyapkumar Apr 17, 2020 •

edited

shreyapkumar Apr 18, 2020

co9olguy Apr 18, 2020

nquesada Apr 17, 2020 •

edited

shreyapkumar Apr 19, 2020

nquesada left a comment

shreyapkumar commented Apr 20, 2020

josh146 left a comment

ishdhand left a comment

shreyapkumar commented Apr 20, 2020

shreyapkumar Apr 20, 2020

josh146 Apr 21, 2020

josh146 left a comment

lneuhaus left a comment

josh146 commented Apr 30, 2020

shreyapkumar commented Apr 30, 2020

	return np.transpose(np.conj(mach_zehnder(m, n, phi_int, phi_ext, nmax)))
	return mach_zehnder(m, n, phi_int, phi_ext, nmax).T.conj()

Hardware-friendly decomposition #363

Hardware-friendly decomposition #363

Conversation

shreyapkumar commented Apr 14, 2020

shreyapkumar commented Apr 14, 2020

codecov bot commented Apr 14, 2020 • edited

Codecov Report

josh146 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josh146 commented Apr 14, 2020 • edited

Redefining name 'T' from outer scope (line 233)

No space allowed before :

Trailing whitespace

No space allowed before :

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shreyapkumar Apr 17, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nquesada Apr 17, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nquesada left a comment

Choose a reason for hiding this comment

shreyapkumar commented Apr 20, 2020

josh146 left a comment

Choose a reason for hiding this comment

ishdhand left a comment

Choose a reason for hiding this comment

shreyapkumar commented Apr 20, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josh146 left a comment

Choose a reason for hiding this comment

lneuhaus left a comment

Choose a reason for hiding this comment

josh146 commented Apr 30, 2020

shreyapkumar commented Apr 30, 2020

codecov bot commented Apr 14, 2020 •

edited

josh146 commented Apr 14, 2020 •

edited

shreyapkumar Apr 17, 2020 •

edited

nquesada Apr 17, 2020 •

edited