Merge pull request #2 from Cryoris/fixrevgrad-public

Use public API in `derive_circuit` only

qiskit_algorithms/gradients/reverse/derive_circuit.py

@@ -16,7 +16,7 @@
 import itertools
 from collections.abc import Sequence
 
-from qiskit.circuit import QuantumCircuit, Parameter, Gate
+from qiskit.circuit import QuantumCircuit, Parameter, Gate, ParameterExpression
 from qiskit.circuit.library import RXGate, RYGate, RZGate, CRXGate, CRYGate, CRZGate
 
 
@@ -90,7 +90,7 @@ def gradient_lookup(gate: Gate) -> list[tuple[complex, QuantumCircuit]]:
 
 
 def derive_circuit(
-    circuit: QuantumCircuit, parameter: Parameter
+    circuit: QuantumCircuit, parameter: Parameter, check: bool = True
 ) -> Sequence[tuple[complex, QuantumCircuit]]:
     """Return the analytic gradient expression of the input circuit wrt. a single parameter.
 
@@ -114,6 +114,8 @@ def derive_circuit(
     Args:
         circuit: The quantum circuit to derive.
         parameter: The parameter with respect to which we derive.
+        check: If ``True`` (default) check that the parameter is valid and that no product
+            rule is required.
 
     Returns:
         A list of ``(coeff, gradient_circuit)`` tuples.
@@ -124,26 +126,31 @@ def derive_circuit(
         NotImplementedError: If a non-unique parameter is added, as the product rule is not yet
             supported in this function.
     """
-    # this is added as useful user-warning, since sometimes ``ParameterExpression``s are
-    # passed around instead of ``Parameter``s
-    if not isinstance(parameter, Parameter):
-        raise ValueError(f"parameter must be of type Parameter, not {type(parameter)}.")
-
-    if parameter not in circuit.parameters:
-        raise ValueError(f"The parameter {parameter} is not in this circuit.")
-
-    if hasattr(circuit, "_parameter_table"):
-        if len(circuit._parameter_table[parameter]) > 1:
-            raise NotImplementedError(
-                "Product rule is not supported, circuit parameters must be unique."
-            )
-    else:
-        # Qiskit is moving circuit functionality to Rust and with the updated logic the former attribute
-        # which was used no longer exists.
-        if circuit._data._get_entry_count(parameter) > 1:
-            raise NotImplementedError(
-                "Product rule is not supported, circuit parameters must be unique."
-            )
+    if check:
+        # this is added as useful user-warning, since sometimes ``ParameterExpression``s are
+        # passed around instead of ``Parameter``s
+        if not isinstance(parameter, Parameter):
+            raise ValueError(f"parameter must be of type Parameter, not {type(parameter)}.")
+
+        if parameter not in circuit.parameters:
+            raise ValueError(f"The parameter {parameter} is not in this circuit.")
+
+        # check uniqueness
+        seen_parameters = set()
+        for instruction in circuit.data:
+            # get parameters in the current operation
+            new_parameters = set()
+            for p in instruction.operation.params:
+                if isinstance(p, ParameterExpression):
+                    new_parameters.update(p.parameters)
+
+            if duplicates := seen_parameters.intersection(new_parameters):
+                raise NotImplementedError(
+                    "Product rule is not supported, circuit parameters must be unique, but "
+                    f"{duplicates} are duplicated."
+                )
+
+            seen_parameters.update(new_parameters)
 
     summands, op_context = [], []
     for i, op in enumerate(circuit.data):

qiskit_algorithms/gradients/reverse/reverse_gradient.py

@@ -144,7 +144,8 @@ def _run_unique(
                 parameter_j = paramlist[j][0]
 
                 # get the analytic gradient d U_j / d p_j and bind the gate
-                deriv = derive_circuit(unitary_j, parameter_j)
+                # we skip the check since we know the circuit has unique, valid parameters
+                deriv = derive_circuit(unitary_j, parameter_j, check=False)
                 for _, gate in deriv:
                     bind(gate, parameter_binds, inplace=True)
 

qiskit_algorithms/gradients/reverse/reverse_qgt.py

@@ -131,7 +131,8 @@ def _run_unique(
             # Note: We currently only support gates with a single parameter -- which is reflected
             # in self.SUPPORTED_GATES -- but generally we could also support gates with multiple
             # parameters per gate. This is the reason for the second 0-index.
-            deriv = derive_circuit(unitaries[0], paramlist[0][0])
+            # We skip the check since we know the circuit has unique, valid parameters.
+            deriv = derive_circuit(unitaries[0], paramlist[0][0], check=False)
             for _, gate in deriv:
                 bind(gate, parameter_binds, inplace=True)
 
@@ -149,7 +150,7 @@ def _run_unique(
                 phi = psi.copy()
 
                 # get the analytic gradient d U_j / d p_j and apply it
-                deriv = derive_circuit(unitaries[j], paramlist[j][0])
+                deriv = derive_circuit(unitaries[j], paramlist[j][0], check=False)
 
                 for _, gate in deriv:
                     bind(gate, parameter_binds, inplace=True)
@@ -170,7 +171,7 @@ def _run_unique(
                     lam = lam.evolve(bound_unitaries[i].inverse())
 
                     # get the gradient d U_i / d p_i and apply it
-                    deriv = derive_circuit(unitaries[i], paramlist[i][0])
+                    deriv = derive_circuit(unitaries[i], paramlist[i][0], check=False)
                     for _, gate in deriv:
                         bind(gate, parameter_binds, inplace=True)
 

qiskit_algorithms/gradients/utils.py

@@ -137,13 +137,6 @@ def _make_lin_comb_gradient_circuit(
                 lin_comb_circuit.h(qr_aux)
                 if add_measurement:
                     lin_comb_circuit.measure(qr_aux, cr_aux)
-                # This next line which assigns data is a workaround otherwise the
-                # circuit parameters may not be properly recognized as data is now
-                # managed in Rust and changing things may break parameters - making a
-                # copy of itself by assignment sorts things out.
-                # See https://github.com/Qiskit/qiskit/blob/main/releasenotes/notes
-                #     /circuit-gates-rust-5c6ab6c58f7fd2c9.yaml#L47-L79
-                lin_comb_circuit.data = lin_comb_circuit.data
                 lin_comb_circuits[p] = lin_comb_circuit
 
     return lin_comb_circuits