Fix bug in single-qubit decomposition edge case. #1643

glassnotes · 2021-09-10T17:01:21Z

Context: Development of #1552 uncovered an edge case in the zyz_decomposition function when the unitary being decomposed contains only off-diagonal elements.

Description of the Change: Fixes the bug, and adds new test cases. I also cleaned up a few parts of the tests so that fewer warnings are spit out by JAX.

Benefits: Fixes bug

Possible Drawbacks: None

Related GitHub Issues: None

github-actions · 2021-09-10T17:01:37Z

Hello. You may have forgotten to update the changelog!
Please edit .github/CHANGELOG.md with:

A one-to-two sentence description of the change. You may include a small working example for new features.
A link back to this PR.
Your name (or GitHub username) in the contributors section.

glassnotes · 2021-09-10T17:02:14Z

tests/transforms/test_decompositions.py

-        assert qml.math.allclose(
-            [x.detach() for x in obtained_gates[0].parameters], expected_params
-        )
+        assert qml.math.allclose(qml.math.unwrap(obtained_gates[0].parameters), expected_params)


The original version caused errors when running locally, so I just tidied up a few things here.

glassnotes · 2021-09-10T17:02:27Z

tests/transforms/test_decompositions.py


    @pytest.mark.parametrize("U,expected_gate,expected_params", single_qubit_decomps)
    def test_zyz_decomposition_jax(self, U, expected_gate, expected_params):
        """Test that a one-qubit operation in JAX is correctly decomposed."""
        jax = pytest.importorskip("jax")

+        # Enable float64 support


Avoids warnings from JAX about precision.

codecov · 2021-09-10T17:16:33Z

Codecov Report

❗ No coverage uploaded for pull request base (v0.18.0-rc0@c3d59e4). Click here to learn what that means.
The diff coverage is n/a.

@@              Coverage Diff               @@
##             v0.18.0-rc0    #1643   +/-   ##
==============================================
  Coverage               ?   99.14%           
==============================================
  Files                  ?      196           
  Lines                  ?    14264           
  Branches               ?        0           
==============================================
  Hits                   ?    14142           
  Misses                 ?      122           
  Partials               ?        0

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

…x` (#1629) * deprecate qml.sample and shots=None for default.qubit.jax * changelog * formatting

josh146

Thanks @glassnotes, nice catch!

josh146 · 2021-09-13T04:56:11Z

pennylane/transforms/decompositions/single_qubit_unitary.py

@@ -16,6 +16,7 @@
 """
 import pennylane as qml
 from pennylane import math
+from pennylane import numpy as np


Best to use standard NumPy here, since it is just being used for pi?

josh146 · 2021-09-13T04:57:05Z

pennylane/transforms/decompositions/single_qubit_unitary.py

-    # If not diagonal, compute the angle of the RY
+    # Check if the matrix is off-diagonal. This means we can express it in terms
+    # of a single Z rotation and a Y rotation of -pi.
+    if math.allclose(U[0, 0], [0.0]):


Does this also imply that U[1, 1] = 0?

Yup, since you can always write a unitary like this:

where a, b are complex numbers, so if a = 0 it must be that its real part is zero (since there's no way to set e^ix = 0), therefore U[1, 1] is 0 as well.

thanks @glassnotes!

josh146 · 2021-09-13T04:57:35Z

pennylane/transforms/decompositions/single_qubit_unitary.py

+        # Otherwise, we need to find the angle of the RZ. Note that we use -pi
+        # as the rotation angle for RY. This ensures the matrices constructed in
+        # PennyLane will match the original.


Oh, was the lack of this negative sign the cause of the bug?

josh146 · 2021-09-13T04:58:36Z

pennylane/transforms/decompositions/single_qubit_unitary.py

+        # PennyLane will match the original.
+        theta = -np.pi
+        phi = 2 * math.angle(U[0, 1])
+        return [qml.Rot(phi, theta, 0.0, wires=wire)]


It strikes me that this is an if statement we wouldn't be able to get rid of for tracing, since it is required to avoid a bug

…I/pennylane into fix_zyz_decomposition

glassnotes · 2021-09-13T12:58:56Z

tests/transforms/test_decompositions.py

    # Next set of gates are non-diagonal and decomposed as Rots
    (H, qml.Rot, [np.pi, np.pi / 2, 0.0]),
-    (X, qml.Rot, [0.0, np.pi, np.pi]),
+    (X, qml.Rot, [0.0, -np.pi, -np.pi]),


This is a result of the new implementation.

glassnotes · 2021-09-13T12:59:20Z

tests/transforms/test_decompositions.py

        assert isinstance(obtained_gates[0], expected_gate)
        assert obtained_gates[0].wires == Wires("a")
        assert qml.math.allclose(obtained_gates[0].parameters, expected_params)

+        if obtained_gates[0].num_params == 1:


I've added matrix equivalence checks to all the tests now... these should have been there from the start 🤦

glassnotes · 2021-09-13T13:01:25Z

@josh146 I was trying to respond to your question about the negative sign, and found a way to streamline the implementation while looking into it. Once the CI runs do you mind taking a second look please?

glassnotes · 2021-09-13T13:18:07Z

[ch8964]

josh146

wow wasn't expecting such a simplification in logic!

josh146 · 2021-09-14T07:15:18Z

pennylane/transforms/decompositions/single_qubit_unitary.py

+        phi = 0.0
+        theta = -np.pi
+        omega = 1j * math.log(U[0, 1] / U[1, 0]) - np.pi


oh this is a massive improvement!

josh146 · 2021-09-14T07:15:45Z

tests/ops/test_qubit_ops.py

            (H, qml.Rot, [np.pi, np.pi / 2, 0]),
-            (X, qml.Rot, [0.0, np.pi, np.pi]),
+            (X, qml.Rot, [0.0, -np.pi, -np.pi]),


Was this because the matrix equivalences were not being checked?

Nope this is just because now for the off-diagonal-only case, theta is by default set to -np.pi.

* Fix bug in zyz decomposition and tidy tests. * Update CHANGELOG. * Tweak comments. * Raise an error for `qml.sample` and `shots=None` in `default.qubit.jax` (#1629) * deprecate qml.sample and shots=None for default.qubit.jax * changelog * formatting * move shot adaptive test * Streamline implementation and add matrix equivalence checks to tests. * Update single qubit op tests. Co-authored-by: Olivia Di Matteo <dimatteo.olivia@gmail.com> Co-authored-by: antalszava <antalszava@gmail.com> Co-authored-by: Josh Izaac <josh146@gmail.com>

* Fix bug in zyz decomposition and tidy tests. * Update CHANGELOG. * Tweak comments. * Raise an error for `qml.sample` and `shots=None` in `default.qubit.jax` (#1629) * deprecate qml.sample and shots=None for default.qubit.jax * changelog * formatting * move shot adaptive test * Streamline implementation and add matrix equivalence checks to tests. * Update single qubit op tests. Co-authored-by: Olivia Di Matteo <dimatteo.olivia@gmail.com> Co-authored-by: antalszava <antalszava@gmail.com> Co-authored-by: Josh Izaac <josh146@gmail.com> Co-authored-by: Olivia Di Matteo <2068515+glassnotes@users.noreply.github.com> Co-authored-by: Olivia Di Matteo <dimatteo.olivia@gmail.com> Co-authored-by: antalszava <antalszava@gmail.com>

* remove newlines in docstring (#1647) * Use numpy. * Fix bug in single-qubit decomposition edge case. (#1643) * Fix bug in zyz decomposition and tidy tests. * Update CHANGELOG. * Tweak comments. * Raise an error for `qml.sample` and `shots=None` in `default.qubit.jax` (#1629) * deprecate qml.sample and shots=None for default.qubit.jax * changelog * formatting * move shot adaptive test * Streamline implementation and add matrix equivalence checks to tests. * Update single qubit op tests. Co-authored-by: Olivia Di Matteo <dimatteo.olivia@gmail.com> Co-authored-by: antalszava <antalszava@gmail.com> Co-authored-by: Josh Izaac <josh146@gmail.com> * Separate torch case. * Update pennylane/devices/default_qubit.py * Fix bug when using Jax jit and `QubitStateVector` (#1649) * Use numpy. * Separate torch case. * Update pennylane/devices/default_qubit.py Co-authored-by: antalszava <antalszava@gmail.com> Co-authored-by: Theodor <theodor@xanadu.ai> Co-authored-by: Romain Moyard <rmoyard@gmail.com> Co-authored-by: Olivia Di Matteo <2068515+glassnotes@users.noreply.github.com> Co-authored-by: Olivia Di Matteo <dimatteo.olivia@gmail.com> Co-authored-by: Josh Izaac <josh146@gmail.com>

* remove newlines in docstring (#1647) * Fix bug in single-qubit decomposition edge case. (#1643) * Fix bug in zyz decomposition and tidy tests. * Update CHANGELOG. * Tweak comments. * Raise an error for `qml.sample` and `shots=None` in `default.qubit.jax` (#1629) * deprecate qml.sample and shots=None for default.qubit.jax * changelog * formatting * move shot adaptive test * Streamline implementation and add matrix equivalence checks to tests. * Update single qubit op tests. Co-authored-by: Olivia Di Matteo <dimatteo.olivia@gmail.com> Co-authored-by: antalszava <antalszava@gmail.com> Co-authored-by: Josh Izaac <josh146@gmail.com> * Fix bug when using Jax jit and `QubitStateVector` (#1649) * Use numpy. * Separate torch case. * Update pennylane/devices/default_qubit.py Co-authored-by: antalszava <antalszava@gmail.com> * Matplotlib circuit drawing (#1484) * Raise a `UserWarning` for a list of shots in `Device.execute` (#1659) * add warning for shot list in pure Device based devices; test for default.gaussian * changelog * PR number * formatting * comment * Update pennylane/_device.py Co-authored-by: Theodor <theodor@xanadu.ai> * test message Co-authored-by: Theodor <theodor@xanadu.ai> * bugfix: raise mpl import error inside class initialization (#1667) * mpldrawer when mpl not installed * black * bugfix for device shot list processing (#1666) * fix process shots bug * add tests and changelog * docstrings and black * add frob to list (#1664) * Small change example. (#1669) * Adds PennyLane-Lightning as a dependency (#1663) * Add pl-lightning to requirements.txt and setup.py * Add to requirements-ci.txt * Temporarily add pybind11 * Update setup.py * Add setup.py * Add to changelog * update changelog * Update changelog * Remove from requirements * Does this fix? * Revert * New version * Add diff method * Add pennylane lightning pypi dep * Update changelog * Add simple test * Remove adjoint * Pin minimum version of lightning * Emphasize as lightning.qubit * Link to PL-Lightning release notes * Update .github/CHANGELOG.md * Update changelog Co-authored-by: Lee J. O'Riordan <loriordan@gmail.com> * tests/transforms/test_optimization/utils.py from rc * tests/transforms/test_decompositions.py from rc * Add shorter drawer example for the notes * Add Hamiltonian expval(H) plot * transforms test files * updated example image Co-authored-by: Theodor <theodor@xanadu.ai> Co-authored-by: Olivia Di Matteo <2068515+glassnotes@users.noreply.github.com> Co-authored-by: Olivia Di Matteo <dimatteo.olivia@gmail.com> Co-authored-by: Josh Izaac <josh146@gmail.com> Co-authored-by: Romain <rmoyard@gmail.com> Co-authored-by: Christina Lee <christina@xanadu.ai> Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com> Co-authored-by: Lee J. O'Riordan <loriordan@gmail.com>

* remove newlines in docstring (#1647) * Fix bug in single-qubit decomposition edge case. (#1643) * Fix bug in zyz decomposition and tidy tests. * Update CHANGELOG. * Tweak comments. * Raise an error for `qml.sample` and `shots=None` in `default.qubit.jax` (#1629) * deprecate qml.sample and shots=None for default.qubit.jax * changelog * formatting * move shot adaptive test * Streamline implementation and add matrix equivalence checks to tests. * Update single qubit op tests. Co-authored-by: Olivia Di Matteo <dimatteo.olivia@gmail.com> Co-authored-by: antalszava <antalszava@gmail.com> Co-authored-by: Josh Izaac <josh146@gmail.com> * Fix bug when using Jax jit and `QubitStateVector` (#1649) * Use numpy. * Separate torch case. * Update pennylane/devices/default_qubit.py Co-authored-by: antalszava <antalszava@gmail.com> * Matplotlib circuit drawing (#1484) * Raise a `UserWarning` for a list of shots in `Device.execute` (#1659) * add warning for shot list in pure Device based devices; test for default.gaussian * changelog * PR number * formatting * comment * Update pennylane/_device.py Co-authored-by: Theodor <theodor@xanadu.ai> * test message Co-authored-by: Theodor <theodor@xanadu.ai> * bugfix: raise mpl import error inside class initialization (#1667) * mpldrawer when mpl not installed * black * bugfix for device shot list processing (#1666) * fix process shots bug * add tests and changelog * docstrings and black * add frob to list (#1664) * Small change example. (#1669) * Adds PennyLane-Lightning as a dependency (#1663) * Add pl-lightning to requirements.txt and setup.py * Add to requirements-ci.txt * Temporarily add pybind11 * Update setup.py * Add setup.py * Add to changelog * update changelog * Update changelog * Remove from requirements * Does this fix? * Revert * New version * Add diff method * Add pennylane lightning pypi dep * Update changelog * Add simple test * Remove adjoint * Pin minimum version of lightning * Emphasize as lightning.qubit * Link to PL-Lightning release notes * Update .github/CHANGELOG.md * Update changelog Co-authored-by: Lee J. O'Riordan <loriordan@gmail.com> * Adjust Torch and TF backprop docstrings (#1662) * error * adjust TF docstring * Diff option backprop check rc (#1658) * update the condition for backprop diff * add test and readjust the other test * formatting * diff_method='parameter-shift' for default.tensor.tf * format tests * Update pennylane/qnode.py Co-authored-by: Romain <rmoyard@gmail.com> Co-authored-by: Romain <rmoyard@gmail.com> * V0.18.0 release notes (#1656) * version bump * group entries * update the descriptions * torch example * contributors * Update .github/CHANGELOG.md Co-authored-by: Josh Izaac <josh146@gmail.com> * Update .github/CHANGELOG.md Co-authored-by: Josh Izaac <josh146@gmail.com> * Update .github/CHANGELOG.md * Update .github/CHANGELOG.md Co-authored-by: Josh Izaac <josh146@gmail.com> * Update .github/CHANGELOG.md Co-authored-by: Josh Izaac <josh146@gmail.com> * Update .github/CHANGELOG.md Co-authored-by: Josh Izaac <josh146@gmail.com> * Update .github/CHANGELOG.md Co-authored-by: Josh Izaac <josh146@gmail.com> * Update .github/CHANGELOG.md Co-authored-by: Josh Izaac <josh146@gmail.com> * Update .github/CHANGELOG.md Co-authored-by: Josh Izaac <josh146@gmail.com> * format lines * num * Update .github/CHANGELOG.md Co-authored-by: Josh Izaac <josh146@gmail.com> * move q transforms * trainable coeffs * SISWAP adjust * Add link to changelog * Fix merge * SISWAP move to improvements * drawer example * remove item on separate requirements file * Update .github/CHANGELOG.md Co-authored-by: Josh Izaac <josh146@gmail.com> * QFT move * further suggestions * NumPy * draw example * no grad call for finite_diff qml.gradients * no grad call for param_shift * lightning * Update .github/CHANGELOG.md Co-authored-by: Josh Izaac <josh146@gmail.com> * Hamiltonian + its plot * Hamiltonian param shift move * bug fix phrasing * contributors list * Lightning adjoint plot & description * Hamiltonian plot width * align mid * Revert "align mid" This reverts commit 9b8d354. Co-authored-by: Josh Izaac <josh146@gmail.com> Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com> Co-authored-by: Theodor <theodor@xanadu.ai> Co-authored-by: Olivia Di Matteo <2068515+glassnotes@users.noreply.github.com> Co-authored-by: Olivia Di Matteo <dimatteo.olivia@gmail.com> Co-authored-by: Josh Izaac <josh146@gmail.com> Co-authored-by: Romain <rmoyard@gmail.com> Co-authored-by: Christina Lee <christina@xanadu.ai> Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com> Co-authored-by: Lee J. O'Riordan <loriordan@gmail.com>

Fix bug in zyz decomposition and tidy tests.

b238b02

glassnotes commented Sep 10, 2021

View reviewed changes

Update CHANGELOG.

64aec27

glassnotes requested a review from josh146 September 10, 2021 17:04

Tweak comments.

ed6d55f

antalszava and others added 2 commits September 11, 2021 22:33

Raise an error for qml.sample and shots=None in `default.qubit.ja…

6003294

…x` (#1629) * deprecate qml.sample and shots=None for default.qubit.jax * changelog * formatting

Merge branch 'master' into fix_zyz_decomposition

7f3b287

josh146 approved these changes Sep 13, 2021

View reviewed changes

josh146 and others added 4 commits September 13, 2021 14:52

move shot adaptive test

3ca661a

Merge branch 'master' into fix_zyz_decomposition

e67f6fd

Streamline implementation and add matrix equivalence checks to tests.

95548c2

Merge branch 'fix_zyz_decomposition' of https://github.com/PennyLaneA…

f28c313

…I/pennylane into fix_zyz_decomposition

glassnotes commented Sep 13, 2021

View reviewed changes

glassnotes requested a review from josh146 September 13, 2021 12:59

Update single qubit op tests.

cb202fb

josh146 approved these changes Sep 14, 2021

View reviewed changes

glassnotes force-pushed the fix_zyz_decomposition branch from eed2775 to cb202fb Compare September 14, 2021 11:52

glassnotes changed the base branch from master to v0.18.0-rc0 September 14, 2021 11:53

Merge branch 'v0.18.0-rc0' into fix_zyz_decomposition

b541563

glassnotes merged commit 0138960 into v0.18.0-rc0 Sep 14, 2021

glassnotes deleted the fix_zyz_decomposition branch September 14, 2021 12:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix bug in single-qubit decomposition edge case. #1643

Fix bug in single-qubit decomposition edge case. #1643

glassnotes commented Sep 10, 2021

github-actions bot commented Sep 10, 2021

glassnotes Sep 10, 2021

glassnotes Sep 10, 2021

codecov bot commented Sep 10, 2021 •

edited

Loading

josh146 left a comment

josh146 Sep 13, 2021

josh146 Sep 13, 2021

glassnotes Sep 13, 2021

josh146 Sep 13, 2021

josh146 Sep 13, 2021

josh146 Sep 13, 2021

glassnotes Sep 13, 2021

glassnotes Sep 13, 2021

glassnotes commented Sep 13, 2021

glassnotes commented Sep 13, 2021

josh146 left a comment

josh146 Sep 14, 2021

josh146 Sep 14, 2021

glassnotes Sep 14, 2021

Fix bug in single-qubit decomposition edge case. #1643

Fix bug in single-qubit decomposition edge case. #1643

Conversation

glassnotes commented Sep 10, 2021

github-actions bot commented Sep 10, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Sep 10, 2021 • edited Loading

Codecov Report

josh146 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

glassnotes commented Sep 13, 2021

glassnotes commented Sep 13, 2021

josh146 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

codecov bot commented Sep 10, 2021 •

edited

Loading