Fix opflow bugs (qiskit-community#1390)

* fix qiskit-community#1311 * fix qiskit-community#1317 * add a test of reps for trotterization * fix tests for qiskit-community#1311 * fix typing * fix qiskit-community#1321 * add release note * fix qiskit-community#1381 * add a test for to_opflow * Update qiskit/aqua/operators/converters/circuit_sampler.py Co-authored-by: John Lapeyre <jlapeyre@users.noreply.github.com> * remove None from constructor * simplify logic * Revert "remove None from constructor" This reverts commit c884ebb. * add releasenote * simplify logic (not iscomplex to isreal) * remove Optional * Update qiskit/aqua/operators/legacy/weighted_pauli_operator.py * Update qiskit/aqua/operators/legacy/weighted_pauli_operator.py * Update qiskit/aqua/operators/legacy/weighted_pauli_operator.py Co-authored-by: John Lapeyre <jlapeyre@users.noreply.github.com> Co-authored-by: Panagiotis Barkoutsos <bpa@zurich.ibm.com> Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
manoelmarques · Nov 9, 2020 · 156bc31 · 156bc31
1 parent 4160cc3
commit 156bc31
Show file tree

Hide file tree

Showing 9 changed files with 84 additions and 14 deletions.
diff --git a/qiskit/aqua/operators/converters/circuit_sampler.py b/qiskit/aqua/operators/converters/circuit_sampler.py
@@ -21,7 +21,7 @@
 from qiskit.providers import Backend
 from qiskit.circuit import QuantumCircuit, Parameter, ParameterExpression
 from qiskit import QiskitError
-from qiskit.aqua import QuantumInstance
+from qiskit.aqua import QuantumInstance, AquaError
 from qiskit.aqua.utils.backend_utils import is_aer_provider, is_statevector_backend
 from qiskit.aqua.operators.operator_base import OperatorBase
 from qiskit.aqua.operators.list_ops.list_op import ListOp
@@ -165,6 +165,8 @@ def convert(self,
 
         Returns:
             The converted Operator with CircuitStateFns replaced by DictStateFns or VectorStateFns.
+        Raises:
+            AquaError: if extracted circuits are empty.
         """
         if self._last_op is None or id(operator) != id(self._last_op):
             # Clear caches
@@ -181,6 +183,11 @@ def convert(self,
         if not self._circuit_ops_cache:
             self._circuit_ops_cache = {}
             self._extract_circuitstatefns(self._reduced_op_cache)
+            if not self._circuit_ops_cache:
+                raise AquaError(
+                    'Circuits are empty. '
+                    'Check that the operator is an instance of CircuitStateFn or its ListOp.'
+                )
 
         if params:
             p_0 = list(params.values())[0]  # type: ignore
@@ -243,8 +250,13 @@ def sample_circuits(self,
 
         Returns:
             The dictionary mapping ids of the CircuitStateFns to their replacement StateFns.
+        Raises:
+            AquaError: if extracted circuits are empty.
         """
-        if circuit_sfns or not self._transpiled_circ_cache:
+        if not circuit_sfns and not self._transpiled_circ_cache:
+            raise AquaError('CircuitStateFn is empty and there is no cache.')
+
+        if circuit_sfns:
             if self._statevector:
                 circuits = [op_c.to_circuit(meas=False) for op_c in circuit_sfns]
             else:

diff --git a/qiskit/aqua/operators/evolutions/trotterizations/trotter.py b/qiskit/aqua/operators/evolutions/trotterizations/trotter.py
@@ -26,4 +26,4 @@ def __init__(self,
         Args:
             reps: The number of times to repeat the Trotterization circuit.
         """
-        super().__init__(order=1, reps=1)
+        super().__init__(order=1, reps=reps)
diff --git a/qiskit/aqua/operators/legacy/weighted_pauli_operator.py b/qiskit/aqua/operators/legacy/weighted_pauli_operator.py
@@ -101,9 +101,19 @@ def to_opflow(self, reverse_endianness=False):
         pauli_ops = []
         for [w, p] in self.paulis:
             pauli = Pauli.from_label(str(p)[::-1]) if reverse_endianness else p
-            # Adding the imaginary is necessary to handle the imaginary coefficients in UCCSD.
-            # TODO fix these or add support for them in Terra.
-            pauli_ops += [PrimitiveOp(pauli, coeff=np.real(w) + np.imag(w))]
+            # This weighted pauli operator has the coeff stored as a complex type
+            # irrespective of whether the value has any imaginary part or not.
+            # For many operators the coeff will be real. Hence below the coeff is made real,
+            # when creating the PrimitiveOp, since it can be stored then as a float, if its
+            # value is real, i.e. has no imaginary part. This avoids any potential issues around
+            # complex - but if there are complex coeffs then maybe that using the opflow
+            # later will fail if it happens to be used where complex is not supported.
+            # Now there are imaginary coefficients in UCCSD that would need to be handled
+            # when this is converted to opflow (evolution of hopping operators) where currently
+            # Terra does not handle complex.
+            # TODO fix these or add support for them in Terra
+            coeff = np.real(w) if np.isreal(w) else w
+            pauli_ops += [PrimitiveOp(pauli, coeff=coeff)]
         return sum(pauli_ops)
 
     @property

diff --git a/qiskit/aqua/operators/state_fns/dict_state_fn.py b/qiskit/aqua/operators/state_fns/dict_state_fn.py
@@ -252,7 +252,7 @@ def sample(self,
                shots: int = 1024,
                massive: bool = False,
                reverse_endianness: bool = False) -> dict:
-        probs = np.array(list(self.primitive.values()))**2
+        probs = np.square(np.abs(np.array(list(self.primitive.values()))))
         unique, counts = np.unique(aqua_globals.random.choice(list(self.primitive.keys()),
                                                               size=shots,
                                                               p=(probs / sum(probs))),

diff --git a/qiskit/aqua/operators/state_fns/operator_state_fn.py b/qiskit/aqua/operators/state_fns/operator_state_fn.py
@@ -34,7 +34,7 @@ class OperatorStateFn(StateFn):
 
     # TODO allow normalization somehow?
     def __init__(self,
-                 primitive: Union[OperatorBase] = None,
+                 primitive: OperatorBase = None,
                  coeff: Union[int, float, complex, ParameterExpression] = 1.0,
                  is_measurement: bool = False) -> None:
         """

diff --git a/releasenotes/notes/fix-operator-flow-33a0bd315151562c.yaml b/releasenotes/notes/fix-operator-flow-33a0bd315151562c.yaml
@@ -0,0 +1,17 @@
+---
+fixes:
+  - |
+    :meth`~qiskit.aqua.operators.state_fns.DictStateFn.sample()` could only handle
+    real amplitudes, but it is fixed to handle complex amplitudes.
+    `#1311 <https://github.com/Qiskit/qiskit-aqua/issues/1311>` for more details.
+  - |
+    Trotter class did not use the reps argument in constructor.
+    `#1317 <https://github.com/Qiskit/qiskit-aqua/issues/1317>` for more details.
+  - |
+    Raise an `AquaError` if :class`qiskit.aqua.operators.converters.CircuitSampler`
+    samples an empty operator.
+    `#1321 <https://github.com/Qiskit/qiskit-aqua/issues/1321>` for more details.
+  - |
+    :meth:`~qiskit.aqua.operators.legacy.WeightedPauliOperator.to_opflow()`
+    returns a correct operator when coefficients are complex numbers.
+    `#1381 <https://github.com/Qiskit/qiskit-aqua/issues/1381>` for more details.
diff --git a/test/aqua/operators/legacy/test_weighted_pauli_operator.py b/test/aqua/operators/legacy/test_weighted_pauli_operator.py
@@ -25,6 +25,7 @@
 from qiskit.aqua.operators import WeightedPauliOperator
 from qiskit.aqua.operators.legacy import op_converter
 from qiskit.aqua.components.initial_states import Custom
+from qiskit.aqua.operators import I, X, Y, Z
 
 
 @ddt
@@ -610,6 +611,22 @@ def eval_op(op):
         expectation_value, _ = eval_op(wpo2)
         self.assertAlmostEqual(expectation_value, -3.0, places=2)
 
+    def test_to_opflow(self):
+        """Test for to_opflow() in WeightedPauliOperator"""
+        pauli_a = 'IXYZ'
+        pauli_b = 'ZYIX'
+        coeff_a = 0.5 + 1j
+        coeff_b = -0.5 - 1j
+        pauli_term_a = [coeff_a, Pauli.from_label(pauli_a)]
+        pauli_term_b = [coeff_b, Pauli.from_label(pauli_b)]
+        op_a = WeightedPauliOperator(paulis=[pauli_term_a])
+        op_b = WeightedPauliOperator(paulis=[pauli_term_b])
+        legacy_op = op_a + op_b
+
+        op = coeff_a * (I ^ X ^ Y ^ Z) + coeff_b * (Z ^ Y ^ I ^ X)
+
+        self.assertEqual(op, legacy_op.to_opflow())
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/test/aqua/operators/test_evolution.py b/test/aqua/operators/test_evolution.py
@@ -22,11 +22,11 @@
 from qiskit.circuit import ParameterVector, Parameter
 
 from qiskit.aqua.operators import (X, Y, Z, I, CX, H, ListOp, CircuitOp, Zero, EvolutionFactory,
-                                   EvolvedOp, PauliTrotterEvolution, QDrift)
-
+                                   EvolvedOp, PauliTrotterEvolution, QDrift, Trotter, Suzuki)
 
 # pylint: disable=invalid-name
 
+
 class TestEvolution(QiskitAquaTestCase):
     """Evolution tests."""
 
@@ -255,6 +255,20 @@ def test_mixed_evolution(self):
         for p in thetas[1:]:
             self.assertNotIn(p, circuit_params)
 
+    def test_reps(self):
+        """Test reps and order params in Trotterization"""
+        reps = 7
+        trotter = Trotter(reps=reps)
+        self.assertEqual(trotter.reps, reps)
+
+        order = 5
+        suzuki = Suzuki(reps=reps, order=order)
+        self.assertEqual(suzuki.reps, reps)
+        self.assertEqual(suzuki.order, order)
+
+        qdrift = QDrift(reps=reps)
+        self.assertEqual(qdrift.reps, reps)
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/test/aqua/operators/test_state_construction.py b/test/aqua/operators/test_state_construction.py
@@ -146,10 +146,10 @@ def test_circuit_state_fn_from_complex_vector_initialize(self):
 
     def test_sampling(self):
         """ state fn circuit from dict initialize test """
-        statedict = {'101': .5,
-                     '100': .1,
-                     '000': .2,
-                     '111': .5}
+        statedict = {'101': .5 + 1.j,
+                     '100': .1 + 2.j,
+                     '000': .2 + 0.j,
+                     '111': .5 + 1.j}
         sfc = CircuitStateFn.from_dict(statedict)
         circ_samples = sfc.sample()
         dict_samples = StateFn(statedict).sample()