[TEMPLATES] Rewrite subroutines as operations #1192

mariaschuld · 2021-04-07T08:46:45Z

This is the fourth PR to refactor the templates (following 1156, 1163 and 1190).

Changes:

Templates are turned into operations, so that the decomposition is executed in the expand function.
The pre-processing was moved into the __init__ function.
The tests were moved into a separate file and extended. Usually, the tests for custom wire labels, shape method and gradients are new, the rest is ported and adapted.
Docstrings were improved.
If the template takes weights, a method to return the shape of the weights was added.

Drawbacks:
Interferometer was not refactored, because an operation of that name already exists. Will make a separate PR with a deprecation warning!

mariaschuld · 2021-04-07T11:59:17Z

pennylane/templates/subroutines/double_excitation_unitary.py

-    Hadamard(wires=r)
-    RX(-np.pi / 2, wires=q)
-    Hadamard(wires=p)
+    qml.Hadamard(wires=s)


Just to be consistent...trying to import as little as possible.

I don't know if this matters here, but having objects in a more local namespace improves speed slightly.

For example, calling CNOT(wires=0,1) is 4% faster than qml.CNOT(wires=(0,1)), but that is a difference of 3.85 microseconds to 4.01 microseconds. It can add up if something gets called many times, but we usually have bigger things to worry about.

mariaschuld · 2021-04-07T11:59:50Z

pennylane/templates/subroutines/double_excitation_unitary.py

-        wires1 (Iterable or Wires): Wires of the qubits representing the subset of occupied orbitals
-            in the interval ``[s, r]``. Accepts an iterable of numbers or strings, or a Wires object,
-            with minimum length 2. The first wire is interpreted as ``s`` and the last wire as ``r``.
+        weight (float or tensor_like): angle :math:`\theta` entering the Z rotation acting on wire ``p``


Added tensor_like, because the scalar could also be a 0-dim tensor...

mariaschuld · 2021-04-07T12:00:13Z

pennylane/templates/subroutines/double_excitation_unitary.py

            the last wire is interpreted as ``p``. Wires in between are acted on with CNOT gates
            to compute the parity of the set of qubits.

-    Raises:
-        ValueError: if inputs do not have the correct format
-


PL does not declare "Raises" anywhere else, so removing them from docstrings

mariaschuld · 2021-04-07T12:00:52Z

pennylane/templates/subroutines/double_excitation_unitary.py

+
+        self.wires1 = list(wires1)
+        self.wires2 = list(wires2)
+        wires = wires1 + wires2


Here I am trying to parse to wires as late as possible (i.e. in the parent function called by super). This is a result of the insight that parsing to Wires is costly...

mariaschuld · 2021-04-07T12:04:01Z

tests/templates/test_subroutines/test_interferometer.py

+import pennylane as qml
+from pennylane.wires import Wires
+
+


These tests were just moved for now...

codecov · 2021-04-07T12:36:43Z

Codecov Report

Merging #1192 (146ec84) into master (8480ab7) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1192   +/-   ##
=======================================
  Coverage   98.12%   98.13%           
=======================================
  Files         145      145           
  Lines       10903    10941   +38     
=======================================
+ Hits        10699    10737   +38     
  Misses        204      204

Impacted Files	Coverage Δ
...ane/templates/subroutines/approx_time_evolution.py	`100.00% <100.00%> (ø)`
...nnylane/templates/subroutines/arbitrary_unitary.py	`100.00% <100.00%> (ø)`
...templates/subroutines/double_excitation_unitary.py	`100.00% <100.00%> (ø)`
pennylane/templates/subroutines/permute.py	`100.00% <100.00%> (ø)`
pennylane/templates/subroutines/qmc.py	`100.00% <100.00%> (ø)`
pennylane/templates/subroutines/qpe.py	`100.00% <100.00%> (ø)`
...templates/subroutines/single_excitation_unitary.py	`100.00% <100.00%> (ø)`
pennylane/templates/subroutines/uccsd.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 8480ab7...146ec84. Read the comment docs.

albi3ro

Looks good!

I left some minor feedback, but nothing that would block merging.

I stopped looking at the tests with as much detail at some point, so I'll trust you on some of that.

albi3ro · 2021-04-07T14:41:21Z

.github/CHANGELOG.md

  from `Operation`, and define the ansatz in their `expand()` method. This 
  change does not affect the user interface.
  [(#1138)](https://github.com/PennyLaneAI/pennylane/pull/1138)
  [(#1156)](https://github.com/PennyLaneAI/pennylane/pull/1156)

-  For convenience, `BasicEntanglerLayers` has a method that returns the shape of the 
-  trainable parameter tensor, i.e.,
+  For convenience, some templates now have a method that returns the expected 


enumerate which ones?

albi3ro · 2021-04-07T15:43:34Z

pennylane/templates/subroutines/double_excitation_unitary.py


-    # Apply the seventh layer
-    _layer7(weight, s, r, q, p, set_cnot_wires)
+            _layer1(weight, s, r, q, p, set_cnot_wires)


Should we make the layer functions members of the class?

I think it's fine this way - they are not user facing anyways.

Actually, these templates may be deprecated soon!

albi3ro · 2021-04-07T16:00:35Z

pennylane/templates/subroutines/double_excitation_unitary.py

-    Hadamard(wires=r)
-    RX(-np.pi / 2, wires=q)
-    Hadamard(wires=p)
+    qml.Hadamard(wires=s)


I don't know if this matters here, but having objects in a more local namespace improves speed slightly.

For example, calling CNOT(wires=0,1) is 4% faster than qml.CNOT(wires=(0,1)), but that is a difference of 3.85 microseconds to 4.01 microseconds. It can add up if something gets called many times, but we usually have bigger things to worry about.

pennylane/templates/subroutines/qpe.py

tests/templates/test_subroutines/test_approx_time_evolution.py

albi3ro · 2021-04-07T20:24:17Z

tests/templates/test_subroutines/test_approx_time_evolution.py

+    def test_jax(self, tol):
+        """Tests the jax interface."""
+
+        jax = pytest.importorskip("jax")


👍 got that this time :)

albi3ro · 2021-04-07T20:33:03Z

tests/templates/test_subroutines/test_arbitrary_unitary.py

+
+        res = circuit(weights)
+        res2 = circuit2(weights)
+        assert qml.math.allclose(res, res2, atol=tol, rtol=0)


Says list_and_tuples but only tests for weights as a list. I remember seeing a similar test before that then executed the circuits with:

weights = tuple(weights)

You are right - renamed the test to check a scalar float.

albi3ro · 2021-04-07T20:35:21Z

tests/templates/test_subroutines/test_arbitrary_unitary.py

+
+        res = circuit(weights)
+        res2 = circuit2(weights)
+        assert qml.math.allclose(res, res2, atol=tol, rtol=0)


Why do some use qml.math.allclose but other use np.allclose?

Because the results of the circuits are tensor objects in the interface that the test used :/

mariaschuld · 2021-04-08T06:17:56Z

Thanks @albi3ro.

About this comment, I did not know about the speed difference. Will keep that in mind!

mariaschuld added 2 commits April 7, 2021 10:02

transformed approx time evol template

86d0991

rewrite arbitrary_unitary

d0b6b1c

mariaschuld marked this pull request as draft April 7, 2021 08:46

mariaschuld added 3 commits April 7, 2021 11:00

interferometer tests moved

105f723

port permute

b58d4b4

ucssd ported

6c2dede

mariaschuld commented Apr 7, 2021

View reviewed changes

mariaschuld changed the title ~~[WIP] [TEMPLATES] Rewrite subroutines as operations~~ [TEMPLATES] Rewrite subroutines as operations Apr 7, 2021

fix qaoa tests

c5e2f71

mariaschuld marked this pull request as ready for review April 7, 2021 12:27

mariaschuld requested review from albi3ro and soranjh April 7, 2021 12:27

albi3ro approved these changes Apr 7, 2021

View reviewed changes

mariaschuld added 5 commits April 8, 2021 08:34

import gates in files

37db465

black

1b06843

update autosummary in docs

18a2a25

fix merge confluct

3f83368

fix test

146ec84

mariaschuld merged commit 4ece39a into master Apr 8, 2021

mariaschuld deleted the rewrite-subroutines branch April 8, 2021 07:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TEMPLATES] Rewrite subroutines as operations #1192

[TEMPLATES] Rewrite subroutines as operations #1192

mariaschuld commented Apr 7, 2021 •

edited

Loading

mariaschuld Apr 7, 2021

albi3ro Apr 7, 2021

mariaschuld Apr 7, 2021

mariaschuld Apr 7, 2021

mariaschuld Apr 7, 2021

mariaschuld Apr 7, 2021 •

edited

Loading

codecov bot commented Apr 7, 2021 •

edited

Loading

albi3ro left a comment

albi3ro Apr 7, 2021

albi3ro Apr 7, 2021

mariaschuld Apr 8, 2021

albi3ro Apr 7, 2021

albi3ro Apr 7, 2021

albi3ro Apr 7, 2021

mariaschuld Apr 8, 2021

albi3ro Apr 7, 2021

mariaschuld Apr 8, 2021

mariaschuld commented Apr 8, 2021

[TEMPLATES] Rewrite subroutines as operations #1192

[TEMPLATES] Rewrite subroutines as operations #1192

Conversation

mariaschuld commented Apr 7, 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

mariaschuld Apr 7, 2021 • edited Loading

Choose a reason for hiding this comment

codecov bot commented Apr 7, 2021 • edited Loading

Codecov Report

albi3ro 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

mariaschuld commented Apr 8, 2021

mariaschuld commented Apr 7, 2021 •

edited

Loading

mariaschuld Apr 7, 2021 •

edited

Loading

codecov bot commented Apr 7, 2021 •

edited

Loading