Add OrbitalRotation operation #1665

obliviateandsurrender · 2021-09-16T23:42:54Z

Context:
This is the first part of the PRs aimed at adding the GateFabric template, which is based on the local, expressive, and quantum number preserving ansatz presented in arXiv:2104.05695.

Description of the Change:
Function OrbitalRotation provides the implementation for the spin-adapted spatial orbital rotation gate which constitutes the gate required for developing the aforementioned template.
Relevant UnitTests for OrbitalRotation operation.

Benefits:
This operation will be required in the implementation of the GateFabric template.

Possible Drawbacks:
No specific drawbacks.

Related GitHub Issues:
No related issues.

ixfoduap

Looks great! Amazing job!

I didn't find any issues, so I'm happy to approve from my side once a small change in the docstrings is made and we have confirmation that the tests are passing.

ixfoduap · 2021-09-17T15:39:37Z

pennylane/ops/qubit/qchem_ops.py

+    This operation performs a Givens rotation between the orbital bases in the Jordan-Wigner basis using a pair
+    of parallel Givens gates :math:`G(\phi)`, which are equivalent to :class:`~.SingleExcitation()` operation.
+    More precisely, for two neighbouring spatial orbitals :math:`\{|\Phi_{0}\rangle, |\Phi_{1}\rangle\}`,
+    it performs the following transformation


Suggested change

This operation performs a Givens rotation between the orbital bases in the Jordan-Wigner basis using a pair

of parallel Givens gates :math:`G(\phi)`, which are equivalent to :class:`~.SingleExcitation()` operation.

More precisely, for two neighbouring spatial orbitals :math:`\{|\Phi_{0}\rangle, |\Phi_{1}\rangle\}`,

it performs the following transformation

For two neighbouring spatial orbitals :math:`\{|\Phi_{0}\rangle, |\Phi_{1}\rangle\}`, this operation

performs the following transformation

Updated in the docstring.

ixfoduap · 2021-09-17T15:40:03Z

pennylane/ops/qubit/qchem_ops.py

+    More precisely, for two neighbouring spatial orbitals :math:`\{|\Phi_{0}\rangle, |\Phi_{1}\rangle\}`,
+    it performs the following transformation
+
+    .. math::


Really clever way to summarize what the operation is doing! 🎉 🧠

I agree!! really nice docstring 🙂

One suggestion: it would be nice to briefly describe the image (in this case, I wasn't originally sure what G(theta) referred to, I assume givens rotations?)

Actually, now that I look at it again: it might be clearer to remove the single-excitation image, and in the text below, simply say

:math:`G(\phi)` represents a single-excitation Given's rotation, implemented in PennyLane as the :class:`~.SingleExcitation` operation.

This way a reader can easily click on the link to read more about the SingleExcitation operation.

I agree. I have added this to the documentation now.

ixfoduap · 2021-09-17T15:40:22Z

pennylane/ops/qubit/qchem_ops.py

+
+    * Number of wires: 4
+    * Number of parameters: 1
+    * Gradient recipe: The ``OrbitalRotation`` operator satisfies a four-term parameter-shift rule


Suggested change

* Gradient recipe: The ``OrbitalRotation`` operator satisfies a four-term parameter-shift rule

* Gradient recipe: The ``OrbitalRotation`` operator satisfies the four-term parameter-shift rule

Updated in the docstring.

ixfoduap · 2021-09-17T15:42:55Z

pennylane/ops/qubit/qchem_ops.py

+        # Additionally, there was a typo in the sign of a matrix element "s" at [2, 8], which is fixed here.
+
+        phi = params[0]
+        c = qml.math.cos(phi / 2)


Great to see qml.math in action!

Same! it is really nice not needing to modify tf_ops.py etc.

ixfoduap · 2021-09-17T15:57:08Z

tests/ops/test_qubit_ops.py

+        pytest.importorskip("autograd")
+
+        dev = qml.device("default.qubit.autograd", wires=4)
+        state = np.array(


How did you compute this state as to avoid using the OrbitalRotation operation? i.e., how do you know it's the correct state?

I'd computed this state manually using the matrix definition of the gate (and cross-checked it with the mentioned transformation).

codecov · 2021-09-17T16:08:42Z

Codecov Report

Merging #1665 (8005ab6) into master (6d93cc5) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1665   +/-   ##
=======================================
  Coverage   99.19%   99.19%           
=======================================
  Files         199      199           
  Lines       14835    14857   +22     
=======================================
+ Hits        14715    14737   +22     
  Misses        120      120

Impacted Files	Coverage Δ
pennylane/devices/default_mixed.py	`100.00% <ø> (ø)`
pennylane/devices/default_qubit.py	`100.00% <ø> (ø)`
pennylane/ops/qubit/__init__.py	`100.00% <ø> (ø)`
pennylane/tape/jacobian_tape.py	`98.19% <ø> (ø)`
pennylane/ops/qubit/qchem_ops.py	`100.00% <100.00%> (ø)`

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 6d93cc5...8005ab6. Read the comment docs.

soranjh · 2021-09-17T17:20:00Z

pennylane/ops/qubit/qchem_ops.py

+
+        >>> @qml.qnode(dev)
+        ... def circuit(phi):
+        ...     qml.BasisState([1, 1, 0, 0], wires=[0, 1, 2, 3])


I think this should be:
qml.BasisState(np.array([1, 1, 0, 0]), wires=[0, 1, 2, 3])

Yeah, I have fixed it now.

soranjh · 2021-09-17T17:20:54Z

tests/ops/test_qubit_ops.py

+            (0.1),
+        ],
+    )
+    def test_autograd_grad(self, phi):


Do we need to test different diff_methods here?

Yes, we should. I have now added the diff_method for this unit test as well.

soranjh

The PR looks good to me @obliviateandsurrender, just left two comments. I am approving as I have already reviewed the new addition in a previous PR.

josh146

Really nice PR @obliviateandsurrender! Left a small comment regarding adding a pytorch gradient test. Apart from that, don't forget to add an entry to the changelog!

josh146 · 2021-09-20T13:51:00Z

pennylane/ops/qubit/qchem_ops.py

+    More precisely, for two neighbouring spatial orbitals :math:`\{|\Phi_{0}\rangle, |\Phi_{1}\rangle\}`,
+    it performs the following transformation
+
+    .. math::


I agree!! really nice docstring 🙂

One suggestion: it would be nice to briefly describe the image (in this case, I wasn't originally sure what G(theta) referred to, I assume givens rotations?)

josh146 · 2021-09-20T13:53:26Z

pennylane/ops/qubit/qchem_ops.py

+    More precisely, for two neighbouring spatial orbitals :math:`\{|\Phi_{0}\rangle, |\Phi_{1}\rangle\}`,
+    it performs the following transformation
+
+    .. math::


Actually, now that I look at it again: it might be clearer to remove the single-excitation image, and in the text below, simply say

:math:`G(\phi)` represents a single-excitation Given's rotation, implemented in PennyLane as the :class:`~.SingleExcitation` operation.

This way a reader can easily click on the link to read more about the SingleExcitation operation.

josh146 · 2021-09-20T13:54:05Z

pennylane/ops/qubit/qchem_ops.py

+        >>> dev = qml.device('default.qubit', wires=4)
+
+        >>> @qml.qnode(dev)
+        ... def circuit(phi):
+        ...     qml.BasisState(np.array([1, 1, 0, 0]), wires=[0, 1, 2, 3])
+        ...     qml.OrbitalRotation(phi, wires=[0, 1, 2, 3])
+        ...     return qml.state()
+
+        >>> circuit(0.1)


Suggested change

>>> dev = qml.device('default.qubit', wires=4)

>>> @qml.qnode(dev)

... def circuit(phi):

... qml.BasisState(np.array([1, 1, 0, 0]), wires=[0, 1, 2, 3])

... qml.OrbitalRotation(phi, wires=[0, 1, 2, 3])

... return qml.state()

>>> circuit(0.1)

>>> dev = qml.device('default.qubit', wires=4)

>>> @qml.qnode(dev)

... def circuit(phi):

... qml.BasisState(np.array([1, 1, 0, 0]), wires=[0, 1, 2, 3])

... qml.OrbitalRotation(phi, wires=[0, 1, 2, 3])

... return qml.state()

>>> circuit(0.1)

josh146 · 2021-09-20T13:55:15Z

pennylane/ops/qubit/qchem_ops.py

+        # Additionally, there was a typo in the sign of a matrix element "s" at [2, 8], which is fixed here.
+
+        phi = params[0]
+        c = qml.math.cos(phi / 2)


Same! it is really nice not needing to modify tf_ops.py etc.

josh146 · 2021-09-20T13:57:35Z

tests/ops/test_qubit_ops.py

+            qml.OrbitalRotation(phi, wires=[0, 1, 2, 3])
+            return qml.expval(qml.PauliZ(0))
+
+        assert np.allclose(jax.grad(circuit)(phi), np.sin(phi))


@obliviateandsurrender could you add a PyTorch test here as well? Aside from that, great tests!

As requested I have added the PyTorch test cases. However, while doing so I faced a problem that the gradient results were returned as None. After spending quite a good amount of time on this and also looking at torch_ops.py, I concluded that defining the matrix for the operations as a single tensor prevents torch to calculate gradients during the backpropagation due to unavailability of the computation graph (as grad_fn was coming out to be None as well). To fix this, I decomposed the matrix definition into the summation of type func(phi)*matrix. Do you think doing this is fine? Let me know.

Hi @obliviateandsurrender, nice catch -- pytorch can be very finicky with how matrices are defined.

No need to modify, but I'm curious if using qml.math.stack to first stack each row of the array, and then stack all rows, would have worked with PyTorch?

Hey @josh146, you're right. Using qml.math.stack indeed works! I think it would be much better to use this instead of the full decomposition.

Awesome! let me know when the master merge conflict is resolved, and I will give it another review :)

Hi! The merge conflict with the master is resolved now.

josh146

Good to go from my end @obliviateandsurrender, once @ixfoduap and @Jaybsoni also approve!

josh146 · 2021-09-21T11:47:03Z

doc/releases/changelog-dev.md

+          -0.04991671+0.j,  0.        +0.j,  0.        +0.j,
+          0.99750208+0.j,  0.        +0.j,  0.        +0.j,
+          0.        +0.j])
+  ```


changelog entry looks 💯

josh146 · 2021-09-21T11:49:10Z

pennylane/ops/qubit/qchem_ops.py

+    More precisely, for two neighbouring spatial orbitals :math:`\{|\Phi_{0}\rangle, |\Phi_{1}\rangle\}`,
+    it performs the following transformation
+
+    .. math::


pennylane/ops/qubit/qchem_ops.py

Co-authored-by: Josh Izaac <josh146@gmail.com>

ixfoduap

doc/releases/changelog-dev.md

…o add-gate-fabric

obliviateandsurrender added 4 commits September 17, 2021 04:58

Added OrbitalRotation operation

700fa1f

Modify varibale name in the unit tests

9de81b1

update docstring

ea3353a

updated docstring

00d168d

ixfoduap requested review from ixfoduap, soranjh, josh146 and Jaybsoni September 17, 2021 15:32

ixfoduap requested changes Sep 17, 2021

View reviewed changes

updated docstring

0882655

soranjh reviewed Sep 17, 2021

View reviewed changes

soranjh approved these changes Sep 17, 2021

View reviewed changes

fixed example and added diff_method

5da0f1f

josh146 requested changes Sep 20, 2021

View reviewed changes

obliviateandsurrender added 5 commits September 21, 2021 03:04

update docs, changelog and added torch grad test

dd320a1

improved inline comments

586f96a

matrix now uses qml.math.stack

7b4f934

fix unnecessary-comprehension

2e1f8c9

Merge branch 'master' into add-gate-fabric

ddedf20

josh146 approved these changes Sep 21, 2021

View reviewed changes

fix unnecessary-list-extraction

93a69a7

Co-authored-by: Josh Izaac <josh146@gmail.com>

ixfoduap approved these changes Sep 21, 2021

View reviewed changes

doc/releases/changelog-dev.md Outdated Show resolved Hide resolved

obliviateandsurrender added 4 commits September 22, 2021 00:05

Merge branch 'master' of https://github.com/PennyLaneAI/pennylane int…

94fc368

…o add-gate-fabric

add OrbitalRotation tests to test_qchem_ops

6a2a478

add OrbitalRotation tests to test_qchem_ops

c8d3f8e

Merge branch 'master' into add-gate-fabric

8005ab6

ixfoduap merged commit 16aac9f into PennyLaneAI:master Sep 22, 2021

	* Gradient recipe: The ``OrbitalRotation`` operator satisfies a four-term parameter-shift rule
	* Gradient recipe: The ``OrbitalRotation`` operator satisfies the four-term parameter-shift rule

Add OrbitalRotation operation #1665

Add OrbitalRotation operation #1665

Conversation

obliviateandsurrender commented Sep 16, 2021

ixfoduap left a comment • 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

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

obliviateandsurrender Sep 17, 2021 • edited Loading

Choose a reason for hiding this comment

codecov bot commented Sep 17, 2021 • edited Loading

Codecov Report

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

soranjh left a comment

Choose a reason for hiding this comment

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

obliviateandsurrender Sep 20, 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

obliviateandsurrender Sep 21, 2021 • edited Loading

Choose a reason for hiding this comment

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

ixfoduap left a comment

Choose a reason for hiding this comment

ixfoduap left a comment •

edited

Loading

obliviateandsurrender Sep 17, 2021 •

edited

Loading

codecov bot commented Sep 17, 2021 •

edited

Loading

obliviateandsurrender Sep 20, 2021 •

edited

Loading

obliviateandsurrender Sep 21, 2021 •

edited

Loading