QuantumCircuit.data modification methods to reset ParameterTable. (#4319

)

Updates `QuantumCircuit.inverse`, `QuantumCircuit.mirror` and the
`QuantumCircuit.data` setter to clear the internal `ParameterTable`
instance prior to rebuilding the `_data` list.

Also, updates `mirror` and `inverse` to prefer `_append` to modifying
`_data` directly, and adds test.python.circuit.test_parameters.
raise_if_parameter_table_invalid to validate if a `ParameterTable` and
its `QuantumCircuit` are consistent.

Additionally, the `inverse method` of the
`qiskit.circuit.library.basis_change.qft` circuit has been updated
to keep the requirement that the returned circuit be a `QFT`
instance.

Resolves #4235 .

Demonstration of issue:

```
>>> qc = qk.QuantumCircuit(1)
>>> x = qk.circuit.Parameter('x')
>>> qc.rz(x, 0)
<qiskit.circuit.instructionset.InstructionSet object at 0x12cb0d780>
>>> qc.data[0]
(<qiskit.extensions.standard.rz.RZGate object at 0x12bd2fb38>, [Qubit(QuantumRegister(1, 'q'), 0)], [])
>>> qc._parameter_table
ParameterTable({Parameter(x): [(<qiskit.extensions.standard.rz.RZGate object at 0x12bd2fb38>, 0)]})
>>>
>>>
>>> inv = qc.inverse()
>>> inv.data[0]
(<qiskit.extensions.standard.rz.RZGate object at 0x12c9c9278>, [Qubit(QuantumRegister(1, 'q'), 0)], [])
>>> inv._parameter_table
ParameterTable({Parameter(x): [(<qiskit.extensions.standard.rz.RZGate object at 0x12cc84da0>, 0)]})
>>>
>>> inv.copy()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/kevin.krsulichibm.com/q/qiskit-terra/qiskit/circuit/quantumcircuit.py", line 1226, in copy
    for param in self._parameter_table
  File "/Users/kevin.krsulichibm.com/q/qiskit-terra/qiskit/circuit/quantumcircuit.py", line 1226, in <dictcomp>
    for param in self._parameter_table
  File "/Users/kevin.krsulichibm.com/q/qiskit-terra/qiskit/circuit/quantumcircuit.py", line 1225, in <listcomp>
    for instr, param_index in self._parameter_table[param]]
KeyError: 5046291872
>>>
>>>
>>> mirror = qc.mirror()
>>> mirror.data[0]
(<qiskit.extensions.standard.rz.RZGate object at 0x12d3c4630>, [Qubit(QuantumRegister(1, 'q'), 0)], [])
>>> mirror._parameter_table
ParameterTable({Parameter(x): [(<qiskit.extensions.standard.rz.RZGate object at 0x12bd2f6a0>, 0), (<qiskit.extensions.standard.rz.RZGate object at 0x12d3c4630>, 0)]})
>>>
>>>
>>> qc.data = []
>>> qc._parameter_table
ParameterTable({Parameter(x): [(<qiskit.extensions.standard.rz.RZGate object at 0x12bd2fb38>, 0)]})
```

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

qiskit/circuit/library/basis_change/qft.py

@@ -218,7 +218,21 @@ def inverse(self) -> 'QFT':
         Returns:
             The inverted circuit.
         """
-        inverted = super().inverse()
+
+        if self.name in ('qft', 'iqft'):
+            name = 'qft' if self._inverse else 'iqft'
+        else:
+            name = self.name + '_dg'
+
+        inverted = self.copy(name=name)
+        inverted._data = []
+
+        from qiskit.circuit.parametertable import ParameterTable
+        inverted._parameter_table = ParameterTable()
+
+        for inst, qargs, cargs in reversed(self._data):
+            inverted._append(inst.inverse(), qargs, cargs)
+
         inverted._inverse = not self._inverse
         return inverted
 

qiskit/circuit/quantumcircuit.py

@@ -197,6 +197,7 @@ def data(self, data_input):
         # below will also empty data_input, so make a shallow copy first.
         data_input = data_input.copy()
         self._data = []
+        self._parameter_table = ParameterTable()
 
         for inst, qargs, cargs in data_input:
             self.append(inst, qargs, cargs)
@@ -252,10 +253,11 @@ def mirror(self):
         Returns:
             QuantumCircuit: the mirrored circuit
         """
-        reverse_circ = self.copy(name=self.name + '_mirror')
-        reverse_circ._data = []
+        reverse_circ = QuantumCircuit(*self.qregs, *self.cregs,
+                                      name=self.name + '_mirror')
+
         for inst, qargs, cargs in reversed(self.data):
-            reverse_circ.append(inst.mirror(), qargs, cargs)
+            reverse_circ._append(inst.mirror(), qargs, cargs)
         return reverse_circ
 
     def inverse(self):
@@ -269,10 +271,11 @@ def inverse(self):
         Raises:
             CircuitError: if the circuit cannot be inverted.
         """
-        inverse_circ = self.copy(name=self.name + '_dg')
-        inverse_circ._data = []
+        inverse_circ = QuantumCircuit(*self.qregs, *self.cregs,
+                                      name=self.name + '_dg')
+
         for inst, qargs, cargs in reversed(self._data):
-            inverse_circ._data.append((inst.inverse(), qargs, cargs))
+            inverse_circ._append(inst.inverse(), qargs, cargs)
         return inverse_circ
 
     def combine(self, rhs):

releasenotes/notes/4235-to_instruction-call-throws-error-on-parametrized-inverted-circuit-acf38e3ddd111900.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    As reported in https://github.com/Qiskit/qiskit-terra/issues/4235 , the
+    ``QuantumCircuit.inverse``, ``QuantumCircuit.mirror`` methods, as well as
+    the ``QuantumCircuit.data`` setter would generate an invalid circuit when
+    used on a parameterized circuit instance. This has been resolved.

test/python/circuit/test_parameters.py

@@ -25,7 +25,8 @@
 import qiskit
 from qiskit import BasicAer
 from qiskit import QuantumRegister, ClassicalRegister, QuantumCircuit
-from qiskit.circuit import Gate, Parameter, ParameterVector, ParameterExpression
+from qiskit.circuit import Gate, Instruction
+from qiskit.circuit import Parameter, ParameterVector, ParameterExpression
 from qiskit.circuit.exceptions import CircuitError
 from qiskit.compiler import assemble, transpile
 from qiskit.execute import execute
@@ -34,6 +35,67 @@
 from qiskit.tools import parallel_map
 
 
+def raise_if_parameter_table_invalid(circuit):  # pylint: disable=invalid-name
+    """Validates the internal consistency of a ParameterTable and its
+    containing QuantumCircuit. Intended for use in testing.
+
+    Raises:
+       CircuitError: if QuantumCircuit and ParameterTable are inconsistent.
+    """
+
+    table = circuit._parameter_table
+
+    # Assert parameters present in circuit match those in table.
+    circuit_parameters = {parameter
+                          for instr, qargs, cargs in circuit._data
+                          for param in instr.params
+                          for parameter in param.parameters
+                          if isinstance(param, ParameterExpression)}
+    table_parameters = set(table._table.keys())
+
+    if circuit_parameters != table_parameters:
+        raise CircuitError('Circuit/ParameterTable Parameter mismatch. '
+                           'Circuit parameters: {}. '
+                           'Table parameters: {}.'.format(
+                               circuit_parameters,
+                               table_parameters))
+
+    # Assert parameter locations in table are present in circuit.
+    circuit_instructions = [instr
+                            for instr, qargs, cargs in circuit._data]
+
+    for parameter, instr_list in table.items():
+        for instr, param_index in instr_list:
+            if instr not in circuit_instructions:
+                raise CircuitError('ParameterTable instruction not present '
+                                   'in circuit: {}.'.format(instr))
+
+            if not isinstance(instr.params[param_index], ParameterExpression):
+                raise CircuitError('ParameterTable instruction does not have a '
+                                   'ParameterExpression at param_index {}: {}.'
+                                   ''.format(param_index, instr))
+
+            if parameter not in instr.params[param_index].parameters:
+                raise CircuitError('ParameterTable instruction parameters does '
+                                   'not match ParameterTable key. Instruction '
+                                   'parameters: {} ParameterTable key: {}.'
+                                   ''.format(instr.params[param_index].parameters,
+                                             parameter))
+
+    # Assert circuit has no other parameter locations other than those in table.
+    for instr, qargs, cargs in circuit._data:
+        for param_index, param in enumerate(instr.params):
+            if isinstance(param, ParameterExpression):
+                parameters = param.parameters
+
+                for parameter in parameters:
+                    if (instr, param_index) not in table[parameter]:
+                        raise CircuitError('Found parameterized instruction not '
+                                           'present in table. Instruction: {} '
+                                           'param_index: {}'.format(instr,
+                                                                    param_index))
+
+
 @ddt
 class TestParameters(QiskitTestCase):
     """QuantumCircuit Operations tests."""
@@ -659,6 +721,46 @@ def test_num_parameters(self):
             qc.x(0)
             self.assertEqual(qc.num_parameters, 0)
 
+    def test_to_instruction_after_inverse(self):
+        """Verify converting an inverse generates a valid ParameterTable"""
+        # ref: https://github.com/Qiskit/qiskit-terra/issues/4235
+        qc = QuantumCircuit(1)
+        theta = Parameter('theta')
+        qc.rz(theta, 0)
+
+        inv_instr = qc.inverse().to_instruction()
+        self.assertIsInstance(inv_instr, Instruction)
+
+    def test_copy_after_inverse(self):
+        """Verify circuit.inverse generates a valid ParameterTable."""
+        qc = QuantumCircuit(1)
+        theta = Parameter('theta')
+        qc.rz(theta, 0)
+
+        inverse = qc.inverse()
+        self.assertIn(theta, inverse.parameters)
+        raise_if_parameter_table_invalid(inverse)
+
+    def test_copy_after_mirror(self):
+        """Verify circuit.mirror generates a valid ParameterTable."""
+        qc = QuantumCircuit(1)
+        theta = Parameter('theta')
+        qc.rz(theta, 0)
+
+        mirror = qc.mirror()
+        self.assertIn(theta, mirror.parameters)
+        raise_if_parameter_table_invalid(mirror)
+
+    def test_copy_after_dot_data_setter(self):
+        """Verify setting circuit.data generates a valid ParameterTable."""
+        qc = QuantumCircuit(1)
+        theta = Parameter('theta')
+        qc.rz(theta, 0)
+
+        qc.data = []
+        self.assertEqual(qc.parameters, set())
+        raise_if_parameter_table_invalid(qc)
+
 
 def _construct_circuit(param, qr):
     qc = QuantumCircuit(qr)