Fix handling of ancillas in functional Pauli rotations

qiskit/circuit/library/arithmetic/integer_comparator.py

@@ -15,9 +15,10 @@
 """Integer Comparator."""
 
 from typing import List, Optional
+import warnings
 import numpy as np
 
-from qiskit.circuit import QuantumRegister
+from qiskit.circuit import QuantumRegister, AncillaRegister
 from qiskit.circuit.exceptions import CircuitError
 from ..boolean_logic import OR
 from ..blueprintcircuit import BlueprintCircuit
@@ -98,6 +99,14 @@ def geq(self, geq: bool) -> None:
             self._invalidate()
             self._geq = geq
 
+    @property
+    def num_ancilla_qubits(self):
+        """Deprecated. Use num_ancillas instead."""
+        warnings.warn('The IntegerComparator.num_ancilla_qubits property is deprecated '
+                      'as of 0.16.0. It will be removed no earlier than 3 months after the release '
+                      'date. You should use the num_ancillas property instead.')
+        return self.num_ancillas
+
     @property
     def num_state_qubits(self) -> int:
         """The number of qubits encoding the state for the comparison.
@@ -120,25 +129,19 @@ def num_state_qubits(self, num_state_qubits: Optional[int]) -> None:
             self._invalidate()  # reset data
             self._num_state_qubits = num_state_qubits
 
-            if num_state_qubits:
+            if num_state_qubits is not None:
                 # set the new qubit registers
-                qr_state = QuantumRegister(self.num_state_qubits, name='state')
+                qr_state = QuantumRegister(num_state_qubits, name='state')
                 q_compare = QuantumRegister(1, name='compare')
 
                 self.qregs = [qr_state, q_compare]
+                self._qubits = qr_state[:] + q_compare[:]
 
-                if self.num_ancilla_qubits > 0:
-                    qr_ancilla = QuantumRegister(self.num_ancilla_qubits, name='ancilla')
-                    self.qregs += [qr_ancilla]
-
-    @property
-    def num_ancilla_qubits(self) -> int:
-        """The number of ancilla qubits used.
-
-        Returns:
-            The number of ancillas in the circuit.
-        """
-        return self._num_state_qubits - 1
+                # add ancillas is required
+                num_ancillas = num_state_qubits - 1
+                if num_ancillas > 0:
+                    qr_ancilla = AncillaRegister(num_ancillas)
+                    self.add_register(qr_ancilla)
 
     def _get_twos_complement(self) -> List[int]:
         """Returns the 2's complement of ``self.value`` as array.

qiskit/circuit/library/arithmetic/piecewise_linear_pauli_rotations.py

@@ -15,9 +15,10 @@
 """Piecewise-linearly-controlled rotation."""
 
 from typing import List, Optional
+import warnings
 import numpy as np
 
-from qiskit.circuit import QuantumRegister
+from qiskit.circuit import QuantumRegister, AncillaRegister
 from qiskit.circuit.exceptions import CircuitError
 
 from .functional_pauli_rotations import FunctionalPauliRotations
@@ -72,6 +73,15 @@ def __init__(self,
 
         super().__init__(num_state_qubits=num_state_qubits, basis=basis, name=name)
 
+    @property
+    def num_ancilla_qubits(self):
+        """Deprecated. Use num_ancillas instead."""
+        warnings.warn('The PiecewiseLinearPauliRotations.num_ancilla_qubits property is deprecated '
+                      'as of 0.16.0. It will be removed no earlier than 3 months after the release '
+                      'date. You should use the num_ancillas property instead.',
+                      DeprecationWarning, stacklevel=2)
+        return self.num_ancillas
+
     @property
     def breakpoints(self) -> List[int]:
         """The breakpoints of the piecewise linear function.
@@ -186,18 +196,6 @@ def evaluate(self, x: float) -> float:
 
         return y
 
-    @property
-    def num_ancilla_qubits(self) -> int:
-        """The number of ancilla qubits.
-
-        Returns:
-            The number of ancilla qubits in the circuit.
-        """
-        num_ancilla_qubits = self.num_state_qubits - 1 + len(self.breakpoints)
-        if self.contains_zero_breakpoint:
-            num_ancilla_qubits -= 1
-        return num_ancilla_qubits
-
     def _check_configuration(self, raise_on_failure: bool = True) -> bool:
         valid = True
 
@@ -220,23 +218,31 @@ def _check_configuration(self, raise_on_failure: bool = True) -> bool:
         return valid
 
     def _reset_registers(self, num_state_qubits: Optional[int]) -> None:
-        if num_state_qubits:
+        if num_state_qubits is not None:
             qr_state = QuantumRegister(num_state_qubits)
             qr_target = QuantumRegister(1)
             self.qregs = [qr_state, qr_target]
+            self._qubits = qr_state[:] + qr_target[:]
+            self._ancillas = []
 
-            if self.num_ancilla_qubits > 0:
-                qr_ancilla = QuantumRegister(self.num_ancilla_qubits)
-                self.qregs += [qr_ancilla]
+            # add ancillas if required
+            if len(self.breakpoints) > 1:
+                num_ancillas = num_state_qubits - 1 + len(self.breakpoints)
+                if self.contains_zero_breakpoint:
+                    num_ancillas -= 1
+                qr_ancilla = AncillaRegister(num_ancillas)
+                self.add_register(qr_ancilla)
         else:
             self.qregs = []
+            self._qubits = []
+            self._ancillas = []
 
     def _build(self):
         super()._build()
 
         qr_state = self.qubits[:self.num_state_qubits]
         qr_target = [self.qubits[self.num_state_qubits]]
-        qr_ancilla = self.qubits[self.num_state_qubits + 1:]
+        qr_ancilla = self.ancillas
 
         # apply comparators and controlled linear rotations
         for i, point in enumerate(self.breakpoints):
@@ -260,7 +266,7 @@ def _build(self):
                 qr_remaining_ancilla = qr_ancilla[i_compare + 1:]  # take remaining ancillas
 
                 self.append(comp.to_gate(),
-                            qr[:] + qr_remaining_ancilla[:comp.num_ancilla_qubits])
+                            qr[:] + qr_remaining_ancilla[:comp.num_ancillas])
 
                 # apply controlled rotation
                 lin_r = LinearPauliRotations(num_state_qubits=self.num_state_qubits,
@@ -272,4 +278,4 @@ def _build(self):
 
                 # uncompute comparator
                 self.append(comp.to_gate().inverse(),
-                            qr[:] + qr_remaining_ancilla[:comp.num_ancilla_qubits])
+                            qr[:] + qr_remaining_ancilla[:comp.num_ancillas])

qiskit/circuit/library/arithmetic/polynomial_pauli_rotations.py

@@ -13,12 +13,13 @@
 # pylint: disable=no-member
 
 """Polynomially controlled Pauli-rotations."""
+
 import warnings
 from typing import List, Optional, Dict, Sequence
 
 from itertools import product
 
-from qiskit.circuit import QuantumRegister
+from qiskit.circuit import QuantumRegister, AncillaRegister
 from qiskit.circuit.exceptions import CircuitError
 
 from .functional_pauli_rotations import FunctionalPauliRotations
@@ -232,23 +233,28 @@ def reverse(self) -> bool:
         return self._reverse
 
     @property
-    def num_ancilla_qubits(self) -> int:
-        """The number of ancilla qubits in this circuit.
-
-        Returns:
-            The number of ancilla qubits.
-        """
-        return max(1, self.degree - 1)
+    def num_ancilla_qubits(self):
+        """Deprecated. Use num_ancillas instead."""
+        warnings.warn('The PolynomialPauliRotations.num_ancilla_qubits property is deprecated '
+                      'as of 0.16.0. It will be removed no earlier than 3 months after the release '
+                      'date. You should use the num_ancillas property instead.',
+                      DeprecationWarning, stacklevel=2)
+        return self.num_ancillas
 
     def _reset_registers(self, num_state_qubits):
-        if num_state_qubits:
+        if num_state_qubits is not None:
             # set new register of appropriate size
             qr_state = QuantumRegister(num_state_qubits, name='state')
             qr_target = QuantumRegister(1, name='target')
-            qr_ancilla = QuantumRegister(self.num_ancilla_qubits, name='ancilla')
+            num_ancillas = max(1, self.degree - 1)
+            qr_ancilla = AncillaRegister(num_ancillas)
             self.qregs = [qr_state, qr_target, qr_ancilla]
+            self._qubits = qr_state[:] + qr_target[:] + qr_ancilla[:]
+            self._ancillas = qr_ancilla[:]
         else:
             self.qregs = []
+            self._qubits = []
+            self._ancillas = []
 
     def _check_configuration(self, raise_on_failure: bool = True) -> bool:
         valid = True

qiskit/dagcircuit/dagcircuit.py

@@ -81,6 +81,7 @@ class DummyCallableList(list):
             """Dummy class so we can deprecate dag.qubits() and do
             dag.qubits as property.
             """
+
             def __call__(self):
                 warnings.warn('dag.qubits() and dag.clbits() are no longer methods. Use '
                               'dag.qubits and dag.clbits properties instead.', DeprecationWarning,
@@ -486,7 +487,10 @@ def _check_wiremap_validity(self, wire_map, keymap, valmap):
                 raise DAGCircuitError("invalid wire mapping key %s" % kname)
             if v not in valmap:
                 raise DAGCircuitError("invalid wire mapping value %s" % vname)
-            if type(k) is not type(v):
+            # TODO Support mapping from AncillaQubit to Qubit, since AncillaQubits are mapped to
+            # Qubits upon being converted to an Instruction. Until this translation is fixed
+            # and Instructions have a concept of ancilla qubits, this fix is required.
+            if not (isinstance(k, type(v)) or isinstance(v, type(k))):
                 raise DAGCircuitError("inconsistent wire_map at (%s,%s)" %
                                       (kname, vname))
 

releasenotes/notes/ancillas-in-functional-pauli-rots-7b7760dfe9933d76.yaml

@@ -0,0 +1,22 @@
+---
+deprecations:
+  - |
+    The ``num_ancilla_qubits`` property of the ``PiecewiseLinearPauliRotations``
+    and ``PolynomialPauliRotations`` classes has been deprecated in favor of
+    the ``num_ancillas`` attribute, which is the name of the method on the
+    ``QuantumCircuit``
+
+fixes:
+  - |
+    The ``PiecewiseLinearPauliRotations`` class always allocated work qubits
+    for the comparison operations, also if there was only one interval and
+    therefore no comparison necessary. This has been fixed.
+
+  - |
+    Also, upon converting a circuit to an instruction the ``AncillaQubit`` are converted
+    to ``Qubit`` type. Therefore appending a circuit with ancillas to another circuit
+    with matching number of ancillas leads to an inconsistent mapping of ancillas to
+    qubits, which causes an error in the DAG circuit.
+    This has temporarily been fixed by allowing to map ancillas to qubits and vice
+    versa and will be fixed permanently in future by giving instructions a
+    concept of ancillas.

test/python/circuit/test_circuit_registers.py

@@ -184,6 +184,24 @@ def test_ancilla_qubit(self):
             action_qubits = [qubit for qubit in qc.qubits if not isinstance(qubit, AncillaQubit)]
             self.assertEqual(len(action_qubits), 2)
 
+    def test_decomposing_with_boxed_ancillas(self):
+        """Test decomposing a circuit which contains an instruction with ancillas.
+
+        This was a previous bug where the wire-map in the DAG raised an error upon mapping
+        the Qubit type to the AncillaQubit type.
+        """
+        ar = AncillaRegister(1)
+        qr = QuantumRegister(1)
+        qc = QuantumCircuit(qr, ar)
+        qc.cx(0, 1)
+        qc.cx(0, 1)
+
+        qc2 = QuantumCircuit(*qc.qregs)
+        qc2.append(qc, [0, 1])
+        decomposed = qc2.decompose()  # used to raise a DAGCircuitError
+
+        self.assertEqual(decomposed, qc)
+
     def test_qregs_circuit(self):
         """Test getting quantum registers from circuit.
         """

test/python/circuit/test_library.py

@@ -609,7 +609,7 @@ class TestFunctionalPauliRotations(QiskitTestCase):
     def assertFunctionIsCorrect(self, function_circuit, reference):
         """Assert that ``function_circuit`` implements the reference function ``reference``."""
         num_state_qubits = function_circuit.num_state_qubits
-        num_ancilla_qubits = function_circuit.num_ancilla_qubits
+        num_ancilla_qubits = function_circuit.num_ancillas
         circuit = QuantumCircuit(num_state_qubits + 1 + num_ancilla_qubits)
         circuit.h(list(range(num_state_qubits)))
         circuit.append(function_circuit.to_instruction(), list(range(circuit.num_qubits)))
@@ -732,6 +732,11 @@ def pw_linear(x):
 
         self.assertFunctionIsCorrect(pw_linear_rotations, pw_linear)
 
+    def test_piecewise_linear_rotations_no_comparator(self):
+        """Test that no ancillas are allocated if not required."""
+        pw_linear_rotations = PiecewiseLinearPauliRotations(3, [0], [1], [0])
+        self.assertEqual(pw_linear_rotations.num_ancillas, 0)
+
     def test_piecewise_linear_rotations_mutability(self):
         """Test the mutability of the linear rotations circuit."""