Repository: PennyLaneAI/pennylane (python)

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.



Pull request description:
fix commuting evolution bug
Fixes #2520

There was some confusion as to the parameters, and during expansion, both `ApproxTimeEvolution` and its decomposition were being queued.

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`."""