Update gradient logic for Qiskit Rust circuit data implementation (ba…

…ckport #188) (#198)

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>

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,16 +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 len(circuit._parameter_table[parameter]) > 1:
-        raise NotImplementedError("No product rule support yet, 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[Parameter] = 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):
@@ -151,7 +168,14 @@ def derive_circuit(
         c = complex(1)
         for i, term in enumerate(product_rule_term):
             c *= term[0]
-            summand_circuit.data.append([term[1], *op_context[i]])
+            # Qiskit changed the format of the stored value. The newer Qiskit has this internal
+            # method to go from the older (legacy) format to new. This logic may need updating
+            # at some point if this internal method goes away.
+            if hasattr(summand_circuit.data, "_resolve_legacy_value"):
+                value = summand_circuit.data._resolve_legacy_value(term[1], *op_context[i])
+                summand_circuit.data.append(value)
+            else:
+                summand_circuit.data.append([term[1], *op_context[i]])
         gradient += [(c, summand_circuit.copy())]
 
     return gradient

qiskit_algorithms/gradients/reverse/reverse_gradient.py

@@ -1,6 +1,6 @@
 # This code is part of a Qiskit project.
 #
-# (C) Copyright IBM 2022, 2023.
+# (C) Copyright IBM 2022, 2024.
 #
 # This code is licensed under the Apache License, Version 2.0. You may
 # obtain a copy of this license in the LICENSE.txt file in the root directory
@@ -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

@@ -1,6 +1,6 @@
 # This code is part of a Qiskit project.
 #
-# (C) Copyright IBM 2023.
+# (C) Copyright IBM 2023, 2024.
 #
 # This code is licensed under the Apache License, Version 2.0. You may
 # obtain a copy of this license in the LICENSE.txt file in the root directory
@@ -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

@@ -1,6 +1,6 @@
 # This code is part of a Qiskit project.
 #
-# (C) Copyright IBM 2022, 2023.
+# (C) Copyright IBM 2022, 2024.
 #
 # This code is licensed under the Apache License, Version 2.0. You may
 # obtain a copy of this license in the LICENSE.txt file in the root directory

releasenotes/notes/fix_gradients_qiskit_rust-3d4c873cf2b23175.yaml

@@ -0,0 +1,6 @@
+---
+other:
+  - |
+    Aspects of the gradients internal implementation, which manipulate circuits more
+    directly, have been updated now that circuit data is being handled by Rust so it's
+    compatible with the former Python way as well as the new Qiskit Rust implementation.

test/gradients/test_estimator_gradient.py

@@ -512,6 +512,18 @@ def operations_callback(op):
         with self.subTest(msg="assert result is correct"):
             self.assertAlmostEqual(result.gradients[0].item(), expect, places=5)
 
+    def test_product_rule_check(self):
+        """Test product rule check."""
+        p = Parameter("p")
+        qc = QuantumCircuit(1)
+        qc.rx(p, 0)
+        qc.ry(p, 0)
+
+        from qiskit_algorithms.gradients.reverse.derive_circuit import derive_circuit
+
+        with self.assertRaises(NotImplementedError):
+            _ = derive_circuit(qc, p)
+
 
 if __name__ == "__main__":
     unittest.main()