SummedOp updates & optimization converters to use Opflow (#1059)

* simplify and reduce, add equals to SummedOp, update tests

* directly use new opflow, no need to go via WPO

* update comments and docstrings

* directly use opflow

* don't do equality check in add

* directly use opflow

* change order in reduce

* fix qaoa

* add short test on summed op equality

* rm prints

* use set comparison, rename simplify to collapse_summands

* fix expected value, should be sqrt(2), not 2

* cast coeffs to complex

* add reno on equals

* fix mypy

* fix spell

* fix lint

* dont cast coefficient to complex

leads to problems if the coeff is exponentitated and not supposed to be complex

* use sum instead of reduce

* rm unused import

* move __hash__ to primitive op and base on repr

* use != over not ==

* add summed op test for different primitives

* check for opbase, not summedop

* adress changes from review

* explicitly raise an error upon ListOp input
* return identity op instead of the int 0

* fix spell

* add note that equals is not mathematically sound

Co-authored-by: Manoel Marques <[email protected]>

Author: Cryoris

qiskit/aqua/operators/list_ops/summed_op.py

@@ -14,7 +14,6 @@
 
 """ SummedOp Class """
 
-from functools import reduce
 from typing import List, Union, cast
 
 import numpy as np
@@ -72,9 +71,6 @@ def add(self, other: OperatorBase) -> OperatorBase:
         Returns:
             A ``SummedOp`` equivalent to the sum of self and other.
         """
-        if self == other:
-            return self.mul(2.0)
-
         self_new_ops = self.oplist if self.coeff == 1 \
             else [op.mul(self.coeff) for op in self.oplist]
         if isinstance(other, SummedOp):
@@ -84,10 +80,10 @@ def add(self, other: OperatorBase) -> OperatorBase:
             other_new_ops = [other]
         return SummedOp(self_new_ops + other_new_ops)
 
-    def simplify(self) -> 'SummedOp':
+    def collapse_summands(self) -> 'SummedOp':
         """Return Operator by simplifying duplicate operators.
 
-        E.g., ``SummedOp([2 * X ^ Y, X ^ Y]).simplify() -> SummedOp([3 * X ^ Y])``.
+        E.g., ``SummedOp([2 * X ^ Y, X ^ Y]).collapse_summands() -> SummedOp([3 * X ^ Y])``.
 
         Returns:
             A simplified ``SummedOp`` equivalent to self.
@@ -113,11 +109,23 @@ def simplify(self) -> 'SummedOp':
                     coeffs.append(self.coeff)
         return SummedOp([op * coeff for op, coeff in zip(oplist, coeffs)])  # type: ignore
 
-    # Try collapsing list or trees of Sums.
     # TODO be smarter about the fact that any two ops in oplist could be evaluated for sum.
     def reduce(self) -> OperatorBase:
-        reduced_ops = [op.reduce() for op in self.oplist]
-        reduced_ops = reduce(lambda x, y: x.add(y), reduced_ops) * self.coeff
+        """Try collapsing list or trees of sums.
+
+        Tries to sum up duplicate operators and reduces the operators
+        in the sum.
+
+        Returns:
+            A collapsed version of self, if possible.
+        """
+        # reduce constituents
+        reduced_ops = sum(op.reduce() for op in self.oplist) * self.coeff
+
+        # group duplicate operators
+        if isinstance(reduced_ops, SummedOp):
+            reduced_ops = reduced_ops.collapse_summands()
+
         if isinstance(reduced_ops, SummedOp) and len(reduced_ops.oplist) == 1:
             return reduced_ops.oplist[0]
         else:
@@ -142,3 +150,49 @@ def to_legacy_op(self, massive: bool = False) -> LegacyBaseOperator:
             coeff = cast(float, self.coeff)
 
         return self.combo_fn(legacy_ops) * coeff
+
+    def equals(self, other: OperatorBase) -> bool:
+        """Check if other is equal to self.
+
+        Note:
+            This is not a mathematical check for equality.
+            If ``self`` and ``other`` implement the same operation but differ
+            in the representation (e.g. different type of summands)
+            ``equals`` will evaluate to ``False``.
+
+        Args:
+            other: The other operator to check for equality.
+
+        Returns:
+            True, if other and self are equal, otherwise False.
+
+        Examples:
+            >>> from qiskit.aqua.operators import X, Z
+            >>> 2 * X == X + X
+            True
+            >>> X + Z == Z + X
+            True
+        """
+        self_reduced, other_reduced = self.reduce(), other.reduce()
+        if not isinstance(other_reduced, type(self_reduced)):
+            return False
+
+        # check if reduced op is still a SummedOp
+        if not isinstance(self_reduced, SummedOp):
+            return self_reduced == other_reduced
+
+        self_reduced = cast(SummedOp, self_reduced)
+        other_reduced = cast(SummedOp, other_reduced)
+        if len(self_reduced.oplist) != len(other_reduced.oplist):
+            return False
+
+        # absorb coeffs into the operators
+        if self_reduced.coeff != 1:
+            self_reduced = SummedOp(
+                [op * self_reduced.coeff for op in self_reduced.oplist])  # type: ignore
+        if other_reduced.coeff != 1:
+            other_reduced = SummedOp(
+                [op * other_reduced.coeff for op in other_reduced.oplist])  # type: ignore
+
+        # compare independent of order
+        return set(self_reduced) == set(other_reduced)

qiskit/aqua/operators/primitive_ops/pauli_op.py

@@ -231,10 +231,6 @@ def exp_i(self) -> OperatorBase:
             from ..evolutions.evolved_op import EvolvedOp
             return EvolvedOp(self)
 
-    def __hash__(self) -> int:
-        # Need this to be able to easily construct AbelianGraphs
-        return hash(str(self))
-
     def commutes(self, other_op: OperatorBase) -> bool:
         """ Returns whether self commutes with other_op.
 

qiskit/aqua/operators/primitive_ops/primitive_op.py

@@ -198,6 +198,9 @@ def log_i(self, massive: bool = False) -> OperatorBase:
     def __str__(self) -> str:
         raise NotImplementedError
 
+    def __hash__(self) -> int:
+        return hash(repr(self))
+
     def __repr__(self) -> str:
         return "{}({}, coeff={})".format(type(self).__name__, repr(self.primitive), self.coeff)
 

qiskit/optimization/converters/ising_to_quadratic_program.py

@@ -15,12 +15,12 @@
 
 """The converter from a ```Operator``` to ``QuadraticProgram``."""
 
-from typing import Optional
+from typing import Optional, Union
 import copy
 
 import numpy as np
 
-from qiskit.aqua.operators.legacy import WeightedPauliOperator
+from qiskit.aqua.operators import OperatorBase, WeightedPauliOperator, SummedOp, ListOp
 from ..problems.quadratic_program import QuadraticProgram
 from ..exceptions import QiskitOptimizationError
 
@@ -44,7 +44,8 @@ def __init__(self, linear: bool = False) -> None:
         self._qp = None  # type: Optional[QuadraticProgram]
         self._linear = linear
 
-    def encode(self, qubit_op: WeightedPauliOperator, offset: float = 0.0) -> QuadraticProgram:
+    def encode(self, qubit_op: Union[OperatorBase, WeightedPauliOperator], offset: float = 0.0
+               ) -> QuadraticProgram:
         """Convert a qubit operator and a shift value into a quadratic program
 
         Args:
@@ -58,8 +59,18 @@ def encode(self, qubit_op: WeightedPauliOperator, offset: float = 0.0) -> Quadra
         Raises:
             QiskitOptimizationError: If there are Pauli Xs in any Pauli term
             QiskitOptimizationError: If there are more than 2 Pauli Zs in any Pauli term
+            NotImplementedError: If the input operator is a ListOp
         """
         # Set properties
+        if isinstance(qubit_op, WeightedPauliOperator):
+            qubit_op = qubit_op.to_opflow()
+
+        # No support for ListOp yet, this can be added in future
+        # pylint: disable=unidiomatic-typecheck
+        if type(qubit_op) == ListOp:
+            raise NotImplementedError('Conversion of a ListOp is not supported, convert each '
+                                      'operator in the ListOp separately.')
+
         self._qubit_op = qubit_op
         self._offset = copy.deepcopy(offset)
         self._num_qubits = qubit_op.num_qubits
@@ -134,24 +145,31 @@ def _create_qubo_matrix(self):
         # The other elements in the QUBO matrix is for quadratic terms of the qubit operator
         self._qubo_matrix = np.zeros((self._num_qubits, self._num_qubits))
 
-        for pauli in self._qubit_op.paulis:
+        if not isinstance(self._qubit_op, SummedOp):
+            oplist = [self._qubit_op.to_pauli_op()]
+        else:
+            oplist = self._qubit_op.to_pauli_op().oplist
+
+        for pauli_op in oplist:
+            pauli = pauli_op.primitive
+            coeff = pauli_op.coeff
             # Count the number of Pauli Zs in a Pauli term
-            lst_z = pauli[1].z.tolist()
+            lst_z = pauli.z.tolist()
             z_index = [i for i, z in enumerate(lst_z) if z is True]
             num_z = len(z_index)
 
             # Add its weight of the Pauli term to the corresponding element of QUBO matrix
             if num_z == 1:
-                self._qubo_matrix[z_index[0], z_index[0]] = pauli[0].real
+                self._qubo_matrix[z_index[0], z_index[0]] = coeff.real
             elif num_z == 2:
-                self._qubo_matrix[z_index[0], z_index[1]] = pauli[0].real
+                self._qubo_matrix[z_index[0], z_index[1]] = coeff.real
             else:
                 raise QiskitOptimizationError(
-                    'There are more than 2 Pauli Zs in the Pauli term {}'.format(pauli[1].z)
+                    'There are more than 2 Pauli Zs in the Pauli term {}'.format(pauli.z)
                 )
 
             # If there are Pauli Xs in the Pauli term, raise an error
-            lst_x = pauli[1].x.tolist()
+            lst_x = pauli.x.tolist()
             x_index = [i for i, x in enumerate(lst_x) if x is True]
             if len(x_index) > 0:
-                raise QiskitOptimizationError('Pauli Xs exist in the Pauli {}'.format(pauli[1].x))
+                raise QiskitOptimizationError('Pauli Xs exist in the Pauli {}'.format(pauli.x))

qiskit/optimization/converters/quadratic_program_to_ising.py

@@ -20,7 +20,7 @@
 import numpy as np
 from qiskit.quantum_info import Pauli
 
-from qiskit.aqua.operators import WeightedPauliOperator
+from qiskit.aqua.operators import OperatorBase, PauliOp, I
 
 from ..problems.quadratic_program import QuadraticProgram
 from ..exceptions import QiskitOptimizationError
@@ -33,7 +33,7 @@ def __init__(self) -> None:
         """Initialize the internal data structure."""
         self._src = None  # type: Optional[QuadraticProgram]
 
-    def encode(self, op: QuadraticProgram) -> Tuple[WeightedPauliOperator, float]:
+    def encode(self, op: QuadraticProgram) -> Tuple[OperatorBase, float]:
         """Convert a problem into a qubit operator
 
         Args:
@@ -114,6 +114,13 @@ def encode(self, op: QuadraticProgram) -> Tuple[WeightedPauliOperator, float]:
             shift += weight
 
         # Remove paulis whose coefficients are zeros.
-        qubit_op = WeightedPauliOperator(paulis=pauli_list)
+        qubit_op = sum(PauliOp(pauli, coeff=coeff) for coeff, pauli in pauli_list)
+
+        # qubit_op could be the integer 0, in this case return an identity operator of
+        # appropriate size
+        if isinstance(qubit_op, OperatorBase):
+            qubit_op = qubit_op.reduce()
+        else:
+            qubit_op = I ^ num_nodes
 
         return qubit_op, shift

releasenotes/notes/summed-op-compare-3df17046849af9a8.yaml

@@ -0,0 +1,5 @@
+---
+features:
+  - |
+    The ``SummedOp`` does a mathematically more correct check for equality, where 
+    expressions such as ``X + X == 2*X`` and ``X + Z == Z + X`` evaluate to ``True``.

test/aqua/operators/test_op_construction.py

@@ -14,23 +14,26 @@
 
 """ Test Operator construction, including OpPrimitives and singletons. """
 
+
 import unittest
 from test.aqua import QiskitAquaTestCase
 import itertools
 import numpy as np
+from ddt import ddt, data
 
 from qiskit.circuit import QuantumCircuit, QuantumRegister, Instruction
 from qiskit.extensions.exceptions import ExtensionError
 from qiskit.quantum_info.operators import Operator, Pauli
-from qiskit.circuit.library import CZGate
+from qiskit.circuit.library import CZGate, ZGate
 
 from qiskit.aqua.operators import (
-    X, Y, Z, I, CX, T, H, PrimitiveOp, SummedOp, PauliOp, Minus, CircuitOp
+    X, Y, Z, I, CX, T, H, PrimitiveOp, SummedOp, PauliOp, Minus, CircuitOp, MatrixOp
 )
 
 
 # pylint: disable=invalid-name
 
+@ddt
 class TestOpConstruction(QiskitAquaTestCase):
     """Operator Construction tests."""
 
@@ -235,7 +238,7 @@ def test_circuit_permute(self):
         c_op_id = c_op_perm.permute(perm)
         self.assertEqual(c_op, c_op_id)
 
-    def test_summed_op(self):
+    def test_summed_op_reduce(self):
         """Test SummedOp"""
         sum_op = (X ^ X * 2) + (Y ^ Y)  # type: SummedOp
         with self.subTest('SummedOp test 1'):
@@ -250,7 +253,7 @@ def test_summed_op(self):
             self.assertListEqual([str(op.primitive) for op in sum_op], ['XX', 'YY', 'YY'])
             self.assertListEqual([op.coeff for op in sum_op], [2, 1, 1])
 
-        sum_op = sum_op.simplify()
+        sum_op = sum_op.collapse_summands()
         with self.subTest('SummedOp test 2-b'):
             self.assertEqual(sum_op.coeff, 1)
             self.assertListEqual([str(op.primitive) for op in sum_op], ['XX', 'YY'])
@@ -263,7 +266,7 @@ def test_summed_op(self):
             self.assertListEqual([str(op.primitive) for op in sum_op], ['XX', 'YY', 'YY', 'XX'])
             self.assertListEqual([op.coeff for op in sum_op], [2, 1, 1, 2])
 
-        sum_op = sum_op.simplify()
+        sum_op = sum_op.reduce()
         with self.subTest('SummedOp test 3-b'):
             self.assertEqual(sum_op.coeff, 1)
             self.assertListEqual([str(op.primitive) for op in sum_op], ['XX', 'YY'])
@@ -275,7 +278,7 @@ def test_summed_op(self):
             self.assertListEqual([str(op.primitive) for op in sum_op], ['XX', 'YY'])
             self.assertListEqual([op.coeff for op in sum_op], [2, 1])
 
-        sum_op = sum_op.simplify()
+        sum_op = sum_op.collapse_summands()
         with self.subTest('SummedOp test 4-b'):
             self.assertEqual(sum_op.coeff, 1)
             self.assertListEqual([str(op.primitive) for op in sum_op], ['XX', 'YY'])
@@ -288,7 +291,7 @@ def test_summed_op(self):
             self.assertListEqual([str(op.primitive) for op in sum_op], ['XX', 'YY', 'YY'])
             self.assertListEqual([op.coeff for op in sum_op], [4, 2, 1])
 
-        sum_op = sum_op.simplify()
+        sum_op = sum_op.collapse_summands()
         with self.subTest('SummedOp test 5-b'):
             self.assertEqual(sum_op.coeff, 1)
             self.assertListEqual([str(op.primitive) for op in sum_op], ['XX', 'YY'])
@@ -301,7 +304,7 @@ def test_summed_op(self):
             self.assertListEqual([str(op.primitive) for op in sum_op], ['XX', 'YY', 'XX', 'YY'])
             self.assertListEqual([op.coeff for op in sum_op], [4, 2, 2, 1])
 
-        sum_op = sum_op.simplify()
+        sum_op = sum_op.collapse_summands()
         with self.subTest('SummedOp test 6-b'):
             self.assertEqual(sum_op.coeff, 1)
             self.assertListEqual([str(op.primitive) for op in sum_op], ['XX', 'YY'])
@@ -310,11 +313,11 @@ def test_summed_op(self):
         sum_op = SummedOp([X ^ X * 2, Y ^ Y], 2)
         sum_op += sum_op
         with self.subTest('SummedOp test 7-a'):
-            self.assertEqual(sum_op.coeff, 4)
-            self.assertListEqual([str(op.primitive) for op in sum_op], ['XX', 'YY'])
-            self.assertListEqual([op.coeff for op in sum_op], [2, 1])
+            self.assertEqual(sum_op.coeff, 1)
+            self.assertListEqual([str(op.primitive) for op in sum_op], ['XX', 'YY', 'XX', 'YY'])
+            self.assertListEqual([op.coeff for op in sum_op], [4, 2, 4, 2])
 
-        sum_op = sum_op.simplify()
+        sum_op = sum_op.collapse_summands()
         with self.subTest('SummedOp test 7-b'):
             self.assertEqual(sum_op.coeff, 1)
             self.assertListEqual([str(op.primitive) for op in sum_op], ['XX', 'YY'])
@@ -326,12 +329,28 @@ def test_summed_op(self):
             self.assertListEqual([str(op.primitive) for op in sum_op], ['XX', 'YY', 'XX', 'ZZ'])
             self.assertListEqual([op.coeff for op in sum_op], [4, 2, 6, 3])
 
-        sum_op = sum_op.simplify()
+        sum_op = sum_op.collapse_summands()
         with self.subTest('SummedOp test 8-b'):
             self.assertEqual(sum_op.coeff, 1)
             self.assertListEqual([str(op.primitive) for op in sum_op], ['XX', 'YY', 'ZZ'])
             self.assertListEqual([op.coeff for op in sum_op], [10, 2, 3])
 
+    def test_summed_op_equals(self):
+        """Test corner cases of SummedOp's equals function."""
+        with self.subTest('multiplicative factor'):
+            self.assertEqual(2 * X, X + X)
+
+        with self.subTest('commutative'):
+            self.assertEqual(X + Z, Z + X)
+
+        with self.subTest('circuit and paulis'):
+            z = CircuitOp(ZGate())
+            self.assertEqual(Z + z, z + Z)
+
+        with self.subTest('matrix op and paulis'):
+            z = MatrixOp([[1, 0], [0, -1]])
+            self.assertEqual(Z + z, z + Z)
+
     def test_circuit_compose_register_independent(self):
         """Test that CircuitOp uses combines circuits independent of the register.
 
@@ -344,13 +363,14 @@ def test_circuit_compose_register_independent(self):
 
         self.assertEqual(composed.num_qubits, 2)
 
-    def test_pauli_op_hashing(self):
+    @data(Z, CircuitOp(ZGate()), MatrixOp([[1, 0], [0, -1]]))
+    def test_op_hashing(self, op):
         """Regression test against faulty set comparison.
 
         Set comparisons rely on a hash table which requires identical objects to have identical
-        hashes. Thus, the PauliOp.__hash__ should support this requirement.
+        hashes. Thus, the PrimitiveOp.__hash__ should support this requirement.
         """
-        self.assertEqual(set([2*Z]), set([2*Z]))
+        self.assertEqual(set([2 * op]), set([2 * op]))
 
 
 if __name__ == '__main__':