Repository: PennyLaneAI/pennylane

Issue:
[BUG] Derivatives of `qml.CommutingEvolution`
In cleaning warnings from the test suite, I found this warning:

```
templates/test_subroutines/test_commuting_evolution.py::TestGradients::test_differentiable_hamiltonian
  /Users/christina/Prog/pl/lib/python3.8/site-packages/autograd/tracer.py:14: UserWarning: Output seems independent of input.
    warnings.warn("Output seems independent of input.")
```

The effect of `CommutingEvolution` on the state did not change with the time.  
This problems seems to stem from [commuting_evolution.py Line 160](https://github.com/PennyLaneAI/pennylane/blob/e55f463bcbca1832505d9440d2f5144543f944fe/pennylane/templates/subroutines/commuting_evolution.py#L160):
```
        return qml.templates.ApproxTimeEvolution(hamiltonian, time_and_coeffs[1], 1).decomposition()
```
This line incorrectly indexes into `time_and_coeffs` to try and extract the time.  It should instead use `time_and_coeffs[0]`.

Upon fixing that bug, we now find that parameter shift no longer works for `CommutingEvolution`.  

These two tests start to fail:
```
==================================================================================== short test summary info ====================================================================================
FAILED templates/test_subroutines/test_commuting_evolution.py::TestGradients::test_two_term_case - assert False
FAILED templates/test_subroutines/test_commuting_evolution.py::TestGradients::test_four_term_case - assert False
```

The tests that fail manually specify the frequencies. If the frequencies are specified, the operation sets its own `grad_recipe`.  This `grad_recipe` gives incorrect results.  I have yet to track down the problem with the `grad_recipe` property.


In summary, fixing the bug in the creation of the `ApproxTimeEvolution` operation allows finite difference derivatives and derivatives for operations without specified frequencies to work correctly.  Since the finite-difference derivatives are now correct, we can see that the parameter-shift gradients never worked correctly.



Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/PennyLaneAI/pennylane/pull/2542?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) Report
> Merging [#2542](https://codecov.io/gh/PennyLaneAI/pennylane/pull/2542?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) (1e3fb1c) into [master](https://codecov.io/gh/PennyLaneAI/pennylane/commit/cc6243d34b98be516780fa4f022f2bfae8613403?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) (cc6243d) will **decrease** coverage by `2.25%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##           master    #2542      +/-   ##
==========================================
- Coverage   99.53%   97.27%   -2.26%     
==========================================
  Files         243      243              
  Lines       19436    19284     -152     
==========================================
- Hits        19345    18759     -586     
- Misses         91      525     +434     
```


| [Impacted Files](https://codecov.io/gh/PennyLaneAI/pennylane/pull/2542?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) | Coverage Δ | |
|---|---|---|
| [...ylane/templates/subroutines/commuting\_evolution.py](https://codecov.io/gh/PennyLaneAI/pennylane/pull/2542/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI#diff-cGVubnlsYW5lL3RlbXBsYXRlcy9zdWJyb3V0aW5lcy9jb21tdXRpbmdfZXZvbHV0aW9uLnB5) | `100.00% <100.00%> (ø)` | |
| [pennylane/interfaces/jax.py](https://codecov.io/gh/PennyLaneAI/pennylane/pull/2542/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI#diff-cGVubnlsYW5lL2ludGVyZmFjZXMvamF4LnB5) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [pennylane/interfaces/jax\_jit.py](https://codecov.io/gh/PennyLaneAI/pennylane/pull/2542/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI#diff-cGVubnlsYW5lL2ludGVyZmFjZXMvamF4X2ppdC5weQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [pennylane/devices/default\_qubit\_jax.py](https://codecov.io/gh/PennyLaneAI/pennylane/pull/2542/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI#diff-cGVubnlsYW5lL2RldmljZXMvZGVmYXVsdF9xdWJpdF9qYXgucHk=) | `0.00% <0.00%> (-96.67%)` | :arrow_down: |
| [pennylane/qnn/cost.py](https://codecov.io/gh/PennyLaneAI/pennylane/pull/2542/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI#diff-cGVubnlsYW5lL3Fubi9jb3N0LnB5) | `42.85% <0.00%> (-57.15%)` | :arrow_down: |
| [pennylane/transforms/classical\_jacobian.py](https://codecov.io/gh/PennyLaneAI/pennylane/pull/2542/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI#diff-cGVubnlsYW5lL3RyYW5zZm9ybXMvY2xhc3NpY2FsX2phY29iaWFuLnB5) | `77.08% <0.00%> (-22.92%)` | :arrow_down: |
| [pennylane/math/utils.py](https://codecov.io/gh/PennyLaneAI/pennylane/pull/2542/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI#diff-cGVubnlsYW5lL21hdGgvdXRpbHMucHk=) | `84.41% <0.00%> (-15.59%)` | :arrow_down: |
| [pennylane/math/multi\_dispatch.py](https://codecov.io/gh/PennyLaneAI/pennylane/pull/2542/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI#diff-cGVubnlsYW5lL21hdGgvbXVsdGlfZGlzcGF0Y2gucHk=) | `88.48% <0.00%> (-11.52%)` | :arrow_down: |
| [pennylane/math/is\_independent.py](https://codecov.io/gh/PennyLaneAI/pennylane/pull/2542/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI#diff-cGVubnlsYW5lL21hdGgvaXNfaW5kZXBlbmRlbnQucHk=) | `88.50% <0.00%> (-11.50%)` | :arrow_down: |
| [pennylane/collections/sum.py](https://codecov.io/gh/PennyLaneAI/pennylane/pull/2542/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI#diff-cGVubnlsYW5lL2NvbGxlY3Rpb25zL3N1bS5weQ==) | `89.47% <0.00%> (-10.53%)` | :arrow_down: |
| ... and [34 more](https://codecov.io/gh/PennyLaneAI/pennylane/pull/2542/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/PennyLaneAI/pennylane/pull/2542?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/PennyLaneAI/pennylane/pull/2542?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI). Last update [cc6243d...1e3fb1c](https://codecov.io/gh/PennyLaneAI/pennylane/pull/2542?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI).

albi3ro: [sc-18955]

Patch:
diff --git a/doc/releases/changelog-dev.md b/doc/releases/changelog-dev.md
index b56a4e186..121452d2b 100644
--- a/doc/releases/changelog-dev.md
+++ b/doc/releases/changelog-dev.md
@@ -75,6 +75,8 @@
   whose generator was a Hamiltonian.
   [(#2524)](https://github.com/PennyLaneAI/pennylane/pull/2524)
 
+* Fixes a bug with the decomposition of `qml.CommutingEvolution`.
+  [(#2542)](https://github.com/PennyLaneAI/pennylane/pull/2542)
 
 <h3>Deprecations</h3>
 
diff --git a/pennylane/templates/subroutines/commuting_evolution.py b/pennylane/templates/subroutines/commuting_evolution.py
index 0ce68ed2f..3ed00c102 100644
--- a/pennylane/templates/subroutines/commuting_evolution.py
+++ b/pennylane/templates/subroutines/commuting_evolution.py
@@ -135,7 +135,7 @@ class CommutingEvolution(Operation):
 
     @staticmethod
     def compute_decomposition(
-        *time_and_coeffs, wires, hamiltonian, **kwargs
+        time, *coeffs, wires, hamiltonian, **kwargs
     ):  # pylint: disable=arguments-differ,unused-argument
         r"""Representation of the operator as a product of other operators.
 
@@ -157,8 +157,8 @@ class CommutingEvolution(Operation):
             list[.Operator]: decomposition of the operator
         """
         # uses standard PauliRot decomposition through ApproxTimeEvolution.
-        hamiltonian = qml.Hamiltonian(time_and_coeffs[1:], hamiltonian.ops)
-        return qml.templates.ApproxTimeEvolution(hamiltonian, time_and_coeffs[1], 1).decomposition()
+        hamiltonian = qml.Hamiltonian(coeffs, hamiltonian.ops)
+        return qml.ApproxTimeEvolution(hamiltonian, time, 1)
 
     def adjoint(self):  # pylint: disable=arguments-differ
 


Test patch:
diff --git a/tests/templates/test_subroutines/test_commuting_evolution.py b/tests/templates/test_subroutines/test_commuting_evolution.py
index 57cbc3333..d115ac7da 100644
--- a/tests/templates/test_subroutines/test_commuting_evolution.py
+++ b/tests/templates/test_subroutines/test_commuting_evolution.py
@@ -18,6 +18,8 @@ import pytest
 from pennylane import numpy as np
 import pennylane as qml
 
+from scipy.linalg import expm
+
 
 def test_adjoint():
     """Tests the CommutingEvolution.adjoint method provides the correct adjoint operation."""
@@ -51,6 +53,58 @@ def test_adjoint():
     assert all(np.isclose(dev1.state, dev2.state))
 
 
+def test_decomposition_expand():
+    """Test that the decomposition of CommutingEvolution is an ApproxTimeEvolution with one step."""
+
+    hamiltonian = 0.5 * qml.PauliX(0) @ qml.PauliY(1)
+    time = 2.345
+
+    op = qml.CommutingEvolution(hamiltonian, time)
+
+    decomp = op.decomposition()
+
+    assert isinstance(decomp, qml.ApproxTimeEvolution)
+    assert all(decomp.hyperparameters["hamiltonian"].coeffs == hamiltonian.coeffs)
+    assert decomp.hyperparameters["n"] == 1
+
+    tape = op.expand()
+    assert len(tape) == 1
+    assert isinstance(tape[0], qml.ApproxTimeEvolution)
+
+
+def test_matrix():
+    """Test that the matrix of commuting evolution is the same as exponentiating -1j * t the hamiltonian."""
+
+    h = 2.34 * qml.PauliX(0)
+    time = 0.234
+    op = qml.CommutingEvolution(h, time)
+
+    mat = qml.matrix(op)
+
+    expected = expm(-1j * time * qml.matrix(h))
+
+    assert qml.math.allclose(mat, expected)
+
+
+def test_forward_execution():
+    """Compare the foward execution to an exactly known result."""
+    dev = qml.device("default.qubit", wires=2)
+
+    H = qml.PauliX(0) @ qml.PauliY(1) - 1.0 * qml.PauliY(0) @ qml.PauliX(1)
+    freq = (2, 4)
+
+    @qml.qnode(dev, diff_method=None)
+    def circuit(time):
+        qml.PauliX(0)
+        qml.CommutingEvolution(H, time, freq)
+        return qml.expval(qml.PauliZ(0))
+
+    t = 1.0
+    res = circuit(t)
+    expected = -np.cos(4)
+    assert np.allclose(res, expected)
+
+
 class TestInputs:
     """Tests for input validation of `CommutingEvolution`."""