Repository: PennyLaneAI/pennylane
Pull request #5133 (2024-01-31 17:05:43)

[Terms2] Add `Sum.terms()` method
Builds on top of https://github.com/PennyLaneAI/pennylane/pull/5132 and adds a meaningul `Sum.terms()` method that returns a tuple of coefficients and ops to be at feature parity with `qml.Hamiltonian`

- [x] Basic functionality
- [x] Tests with Paulis
- [x] Tests without Paulis
- [x] Docs
- [x] changelog
- [x] handle empty pauli words

github-actions[bot]: Hello. You may have forgotten to update the changelog!
Please edit [`doc/releases/changelog-dev.md`](/PennyLaneAI/pennylane/blob/master/doc/releases/changelog-dev.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.
Qottmann: [sc-55297]
Qottmann: Some rough numbers to see where we are with this
```
data = qml.data.load("qchem", molname="BH3", bondlength=1.189, basis="STO-3G")[0]
H1 = data.hamiltonian

%timeit coeffs, ops = H1.terms()
H1_Sum = qml.dot(*H1.terms())
%timeit coeffs, ops = H1_Sum.terms()
```
```
3.25 µs ± 152 ns per loop (mean ± std. dev. of 7 runs, 100,000 loops each)
99.4 ms ± 2.85 ms per loop (mean ± std. dev. of 7 runs, 10 loops each)
```

```
data = qml.data.load("qchem", molname="C2H2", bondlength=1.203, basis="STO-3G")[0]
H2 = data.hamiltonian

%timeit coeffs, ops = H1.terms()
H2_Sum = qml.dot(*H1.terms())
%timeit coeffs, ops = H1_Sum.terms()
```
```
3.04 µs ± 6.35 ns per loop (mean ± std. dev. of 7 runs, 100,000 loops each)
98.8 ms ± 351 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)
```

It is significantly slower, not sure how to improve that though 🤔
albi3ro: @Qottmann might be time to prioritize the changes from this branch https://github.com/PennyLaneAI/pennylane/tree/conversion-timings if converting from pauli rep to operations are a blocker.
Qottmann: > if converting from pauli rep to operations are a blocker.

I dont think that is the case here or am I missing something 😬 ?
albi3ro: 99.79% of the time in `Sum.terms()` is being spent in `PauliSentence.operation()`.

<img width="1051" alt="Screenshot 2024-02-01 at 12 01 48 PM" src="https://github.com/PennyLaneAI/pennylane/assets/6364575/7574fd2e-bfbe-483a-9623-39263ca75713">

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/PennyLaneAI/pennylane/pull/5133?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) Report
Attention: `10 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`0305861`)](https://app.codecov.io/gh/PennyLaneAI/pennylane/commit/0305861805576eeda1ac237d455939a904cecc3c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) 99.68% compared to head [(`3111d2b`)](https://app.codecov.io/gh/PennyLaneAI/pennylane/pull/5133?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) 99.65%.

| [Files](https://app.codecov.io/gh/PennyLaneAI/pennylane/pull/5133?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) | Patch % | Lines |
|---|---|---|
| [pennylane/ops/op\_math/sum.py](https://app.codecov.io/gh/PennyLaneAI/pennylane/pull/5133?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI#diff-cGVubnlsYW5lL29wcy9vcF9tYXRoL3N1bS5weQ==) | 28.57% | [10 Missing :warning: ](https://app.codecov.io/gh/PennyLaneAI/pennylane/pull/5133?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) |

<details><summary>Additional details and impacted files</summary>


```diff
@@              Coverage Diff              @@
##           prodterms    #5133      +/-   ##
=============================================
- Coverage      99.68%   99.65%   -0.03%     
=============================================
  Files            394      394              
  Lines          35859    35871      +12     
=============================================
+ Hits           35747    35749       +2     
- Misses           112      122      +10     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/PennyLaneAI/pennylane/pull/5133?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI).

mudit2812: As also mentioned in #5132 , I no longer think we should cache the terms.
codecov-commenter: ## [Codecov](https://app.codecov.io/gh/PennyLaneAI/pennylane/pull/5133?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) Report
All modified and coverable lines are covered by tests :white_check_mark:
> :exclamation: No coverage uploaded for pull request base (`prodterms@9d7640a`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI#section-missing-base-commit).


<details><summary>Additional details and impacted files</summary>


```diff
@@             Coverage Diff              @@
##             prodterms    #5133   +/-   ##
============================================
  Coverage             ?   99.68%           
============================================
  Files                ?      394           
  Lines                ?    35914           
  Branches             ?        0           
============================================
  Hits                 ?    35801           
  Misses               ?      113           
  Partials             ?        0           
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/PennyLaneAI/pennylane/pull/5133?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI).

trbromley: Looks great to me in principle! What was the resolution to the performance worries? Are the numbers shared above still accurate?
albi3ro: <img width="261" alt="Screenshot 2024-02-09 at 10 35 25 AM" src="https://github.com/PennyLaneAI/pennylane/assets/6364575/ad9f9d66-56bf-4c2c-9e9f-9fb6c722da21">


Patch:
diff --git a/doc/releases/changelog-dev.md b/doc/releases/changelog-dev.md
index 0b0d74a9b..7e61ad918 100644
--- a/doc/releases/changelog-dev.md
+++ b/doc/releases/changelog-dev.md
@@ -221,6 +221,17 @@
   ```
   [(#5132)](https://github.com/PennyLaneAI/pennylane/pull/5132)
 
+* Upgrade the `Sum.terms()` method to return a tuple `(coeffs, ops)` consisting of coefficients and pure product operators.
+  ```python3
+  >>> qml.operation.enable_new_opmath()
+  >>> op = 0.5 * X(0) + 0.7 * X(1) + 1.5 * Y(0) @ Y(1)
+  >>> op.terms()
+  ([0.5, 0.7, 1.5],
+   [X(0), X(1), Y(1) @ Y(0)])
+  ```
+  [(#5133)](https://github.com/PennyLaneAI/pennylane/pull/5133)
+
+
 * String representations of Pauli operators have been improved and there are new aliases `X, Y, Z, I` for `PauliX, PauliY, PauliZ, Identity`.
   ```
   >>> qml.PauliX(0)
diff --git a/pennylane/ops/op_math/sum.py b/pennylane/ops/op_math/sum.py
index a2033fb4e..2d5a7e6dc 100644
--- a/pennylane/ops/op_math/sum.py
+++ b/pennylane/ops/op_math/sum.py
@@ -189,19 +189,6 @@ class Sum(CompositeOp):
 
         return all(s.is_hermitian for s in self)
 
-    def terms(self):
-        r"""Representation of the operator as a linear combination of other operators.
-
-        .. math:: O = \sum_i c_i O_i
-
-        A ``TermsUndefinedError`` is raised if no representation by terms is defined.
-
-        Returns:
-            tuple[list[tensor_like or float], list[.Operation]]: list of coefficients :math:`c_i`
-            and list of operations :math:`O_i`
-        """
-        return [1.0] * len(self), list(self)
-
     def matrix(self, wire_order=None):
         r"""Representation of the operator as a matrix in the computational basis.
 
@@ -314,6 +301,51 @@ class Sum(CompositeOp):
             return Sum(*new_summands) if len(new_summands) > 1 else new_summands[0]
         return qml.s_prod(0, qml.Identity(self.wires))
 
+    def terms(self):
+        r"""Representation of the operator as a linear combination of other operators.
+
+        .. math:: O = \sum_i c_i O_i
+
+        A ``TermsUndefinedError`` is raised if no representation by terms is defined.
+
+        Returns:
+            tuple[list[tensor_like or float], list[.Operation]]: list of coefficients :math:`c_i`
+            and list of operations :math:`O_i`
+
+        **Example**
+
+        >>> qml.operation.enable_new_opmath()
+        >>> op = 0.5 * X(0) + 0.7 * X(1) + 1.5 * Y(0) @ Y(1)
+        >>> op.terms()
+        ([0.5, 0.7, 1.5],
+         [X(0), X(1), Y(1) @ Y(0)])
+
+        Note that this method disentangles nested structures of ``Sum`` instances like so.
+        >>> op = 0.5 * X(0) + (2. * (X(1) + 3. * X(2)))
+        >>> print(op)
+        (0.5*(PauliX(wires=[0]))) + (2.0*((0.5*(PauliX(wires=[1]))) + (3.0*(PauliX(wires=[2])))))
+        >>> print(op.terms())
+        ([0.5, 1.0, 6.0], [PauliX(wires=[0]), PauliX(wires=[1]), PauliX(wires=[2])])
+
+        """
+        # try using pauli_rep:
+        if pr := self.pauli_rep:
+            ops = [pauli.operation() for pauli in pr.keys()]
+            return list(pr.values()), ops
+
+        new_summands = self._simplify_summands(summands=self.operands).get_summands()
+
+        coeffs = []
+        ops = []
+        for factor in new_summands:
+            if isinstance(factor, qml.ops.SProd):
+                coeffs.append(factor.scalar)
+                ops.append(factor.base)
+            else:
+                coeffs.append(1.0)
+                ops.append(factor)
+        return coeffs, ops
+
     @classmethod
     def _sort(cls, op_list, wire_map: dict = None) -> List[Operator]:
         """Sort algorithm that sorts a list of sum summands by their wire indices.


Test patch:
diff --git a/tests/ops/op_math/test_sum.py b/tests/ops/op_math/test_sum.py
index 68582d2a5..16943b117 100644
--- a/tests/ops/op_math/test_sum.py
+++ b/tests/ops/op_math/test_sum.py
@@ -28,6 +28,8 @@ from pennylane.wires import Wires
 from pennylane.operation import AnyWires, MatrixUndefinedError, Operator
 from pennylane.ops.op_math import Prod, Sum
 
+X, Y, Z = qml.PauliX, qml.PauliY, qml.PauliZ
+
 no_mat_ops = (
     qml.Barrier,
     qml.WireCut,
@@ -147,18 +149,115 @@ class TestInitialization:
         assert sum_op.parameters == [0.23, 9.87]
         assert sum_op.num_params == 2
 
-    @pytest.mark.parametrize("ops_lst", ops)
-    def test_terms(self, ops_lst):
-        """Test that terms are initialized correctly."""
-        sum_op = Sum(*ops_lst)
-        coeff, sum_ops = sum_op.terms()
+    coeffs_ = [1.0, 1.0, 1.0, 3.0, 4.0, 4.0, 5.0]
+    h6 = qml.sum(
+        qml.s_prod(2.0, qml.prod(qml.PauliX(0), qml.PauliZ(10))),
+        qml.s_prod(3.0, qml.prod(qml.PauliX(1), qml.PauliZ(11))),
+    )
+    ops_ = [
+        qml.s_prod(1.0, qml.PauliX(0)),
+        qml.s_prod(1.0, qml.prod(qml.PauliX(0), qml.PauliX(1))),
+        qml.s_prod(2.0, qml.prod(qml.PauliZ(0), qml.PauliZ(1))),
+        qml.s_prod(2.0, qml.PauliY(0)),
+        qml.s_prod(2.0, qml.prod(qml.PauliY(0), qml.PauliY(1))),
+        qml.s_prod(2.0, qml.prod(qml.s_prod(2.0, qml.PauliY(2)), qml.PauliY(3))),
+        h6,
+    ]
+    Hamiltonian_with_Paulis = qml.dot(coeffs_, ops_)
+
+    SUM_TERMS_OP_PAIRS_PAULI = (  # all operands have pauli representation
+        (
+            qml.sum(*(i * X(i) for i in range(1, 5))),
+            [float(i) for i in range(1, 5)],
+            [X(i) for i in range(1, 5)],
+        ),
+        (
+            qml.sum(*(qml.s_prod(i, qml.prod(X(i), X(i + 1))) for i in range(1, 5))),
+            [float(i) for i in range(1, 5)],
+            [qml.prod(X(i), X(i + 1)) for i in range(1, 5)],
+        ),
+        (
+            Hamiltonian_with_Paulis,
+            [1.0, 1.0, 2.0, 6.0, 8.0, 16.0, 10.0, 15.0],
+            [
+                X(0),
+                qml.prod(X(1), X(0)),
+                qml.prod(Z(1), Z(0)),
+                Y(0),
+                qml.prod(Y(1), Y(0)),
+                qml.prod(Y(3), Y(2)),
+                qml.prod(Z(10), X(0)),
+                qml.prod(Z(11), X(1)),
+            ],
+        ),
+    )
 
-        assert coeff == [1.0, 1.0, 1.0]
+    @pytest.mark.parametrize("op, coeffs_true, ops_true", SUM_TERMS_OP_PAIRS_PAULI)
+    def test_terms_pauli_rep(self, op, coeffs_true, ops_true):
+        """Test that Sum.terms() is correct for operators that all have a pauli_rep"""
+        coeffs, ops1 = op.terms()
+        assert coeffs == coeffs_true
+        assert ops1 == ops_true
+
+    coeffs_ = [1.0, 1.0, 1.0, 3.0, 4.0, 4.0, 5.0]
+    h6 = qml.sum(
+        qml.s_prod(2.0, qml.prod(qml.Hadamard(0), qml.PauliZ(10))),
+        qml.s_prod(3.0, qml.prod(qml.PauliX(1), qml.PauliZ(11))),
+    )
+    ops_ = [
+        qml.s_prod(1.0, qml.Hadamard(0)),
+        qml.s_prod(1.0, qml.prod(qml.Hadamard(0), qml.PauliX(1))),
+        qml.s_prod(2.0, qml.prod(qml.PauliZ(0), qml.PauliZ(1))),
+        qml.s_prod(2.0, qml.PauliY(0)),
+        qml.s_prod(2.0, qml.prod(qml.PauliY(0), qml.PauliY(1))),
+        qml.s_prod(2.0, qml.prod(qml.s_prod(2.0, qml.PauliY(2)), qml.PauliY(3))),
+        h6,
+    ]
+    Hamiltonian_mixed = qml.dot(coeffs_, ops_)
+
+    SUM_TERMS_OP_PAIRS_MIXEDPAULI = (  # all operands have pauli representation
+        (
+            qml.sum(*(i * qml.Hadamard(i) for i in range(1, 5))),
+            [float(i) for i in range(1, 5)],
+            [qml.Hadamard(i) for i in range(1, 5)],
+        ),
+        (
+            qml.sum(qml.sum(*(i * qml.Hadamard(i) for i in range(1, 5))), 0.0 * qml.Identity(0)),
+            [float(i) for i in range(1, 5)],
+            [qml.Hadamard(i) for i in range(1, 5)],
+        ),
+        (
+            qml.sum(qml.sum(*(i * qml.Hadamard(i) for i in range(1, 5))), qml.Identity(0)),
+            [float(i) for i in range(1, 5)] + [1.0],
+            [qml.Hadamard(i) for i in range(1, 5)] + [qml.Identity(0)],
+        ),
+        (
+            qml.sum(*(qml.s_prod(i, qml.prod(X(i), qml.Hadamard(i + 1))) for i in range(1, 5))),
+            [float(i) for i in range(1, 5)],
+            [qml.prod(X(i), qml.Hadamard(i + 1)) for i in range(1, 5)],
+        ),
+        (
+            Hamiltonian_mixed,
+            [1.0, 1.0, 2.0, 6.0, 8.0, 16.0, 10.0, 15.0],
+            [
+                qml.Hadamard(0),
+                qml.prod(X(1), qml.Hadamard(0)),
+                qml.prod(Z(1), Z(0)),
+                Y(0),
+                qml.prod(Y(1), Y(0)),
+                qml.prod(Y(3), Y(2)),
+                qml.prod(Z(10), qml.Hadamard(0)),
+                qml.prod(Z(11), X(1)),
+            ],
+        ),
+    )
 
-        for op1, op2 in zip(sum_ops, ops_lst):
-            assert op1.name == op2.name
-            assert op1.wires == op2.wires
-            assert op1.data == op2.data
+    @pytest.mark.parametrize("op, coeffs_true, ops_true", SUM_TERMS_OP_PAIRS_MIXEDPAULI)
+    def test_terms_mixed(self, op, coeffs_true, ops_true):
+        """Test that Sum.terms() is correct for operators that dont all have a pauli_rep"""
+        coeffs, ops1 = op.terms()
+        assert coeffs == coeffs_true
+        assert ops1 == ops_true
 
     def test_eigen_caching(self):
         """Test that the eigendecomposition is stored in cache."""