Fixed bug in gradients of loaded qiskit circuits (#71)

* Fixed bug in gradients of loaded qiskit circuits * fix * remove f-string * suggested changes * Apply suggestions from code review Co-Authored-By: antalszava <[email protected]> * add type hint * added test * rename variableref to variable * remove variableref reference Co-authored-by: antalszava <[email protected]>
PennyLaneAI · Feb 28, 2020 · 016fbc1 · 016fbc1
1 parent 803305a
commit 016fbc1
Show file tree

Hide file tree

Showing 4 changed files with 125 additions and 26 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,10 +10,16 @@
 
 ### Bug fixes
 
+* Fixed a bug where gradient computations always returned 0 when
+  loading a parametrized Qiskit circuit as a PennyLane template.
+  [(#71)](https://github.com/XanaduAI/pennylane-qiskit/pull/71)
+
 ### Contributors
 
 This release contains contributions from (in alphabetical order):
 
+Josh Izaac
+
 ---
 
 # Release 0.8.0

diff --git a/pennylane_qiskit/converter.py b/pennylane_qiskit/converter.py
@@ -20,11 +20,12 @@
 This module contains functions for converting Qiskit QuantumCircuit objects
 into PennyLane circuit templates.
 """
+from typing import Dict, Any
 import warnings
 
 import numpy as np
 from qiskit import QuantumCircuit
-from qiskit.circuit import Parameter
+from qiskit.circuit import Parameter, ParameterExpression
 from qiskit.exceptions import QiskitError
 
 import pennylane as qml
@@ -36,40 +37,59 @@
 inv_map = {v.__name__: k for k, v in QISKIT_OPERATION_MAP.items()}
 
 
-def _check_parameter_bound(param):
+def _check_parameter_bound(param: Parameter, var_ref_map: Dict[Parameter, qml.variable.Variable]):
     """Utility function determining if a certain parameter in a QuantumCircuit has
     been bound.
 
     Args:
-        param: the parameter to be checked
+        param (qiskit.circuit.Parameter): the parameter to be checked
+        var_ref_map (dict[qiskit.circuit.Parameter, pennylane.variable.Variable]):
+            a dictionary mapping qiskit parameters to PennyLane variables
+    """
+    if isinstance(param, Parameter) and param not in var_ref_map:
+        raise ValueError("The parameter {} was not bound correctly.".format(param))
+
+
+def _extract_variable_refs(params: Dict[Parameter, Any]) -> Dict[Parameter, qml.variable.Variable]:
+    """Iterate through the parameter mapping to be bound to the circuit,
+    and return a dictionary containing the differentiable parameters.
+
+    Args:
+        params (dict): dictionary of the parameters in the circuit to their corresponding values
 
     Returns:
-        param: the parameter after the check
+        dict[qiskit.circuit.Parameter, pennylane.variable.Variable]: a dictionary mapping
+            qiskit parameters to PennyLane variables
     """
-    if isinstance(param, Parameter):
-        raise ValueError("The parameter {} was not bound correctly.".format(param))
-    return param
+    # map qiskit parameters to PennyLane differentiable Variables.
+    if params is None:
+        return {}
+
+    return {k: v for k, v in params.items() if isinstance(v, qml.variable.Variable)}
 
 
-def _check_circuit_and_bind_parameters(quantum_circuit: QuantumCircuit, params: dict) -> QuantumCircuit:
+def _check_circuit_and_bind_parameters(quantum_circuit: QuantumCircuit, params: dict, diff_params: dict) -> QuantumCircuit:
     """Utility function for checking for a valid quantum circuit and then binding parameters.
 
     Args:
         quantum_circuit (QuantumCircuit): the quantum circuit to check and bind the parameters for
-        params (dict): dictionary of the parameters in the circuit
+        params (dict): dictionary of the parameters in the circuit to their corresponding values
+        diff_params (dict): dictionary mapping the differentiable parameters to PennyLane
+            Variable instances
 
     Returns:
-        qc (QuantumCircuit): quantum circuit with bound parameters
+        QuantumCircuit: quantum circuit with bound parameters
     """
     if not isinstance(quantum_circuit, QuantumCircuit):
         raise ValueError("The circuit {} is not a valid Qiskit QuantumCircuit.".format(quantum_circuit))
 
     if params is None:
         return quantum_circuit
 
-    for k, v in params.items():
-        if isinstance(v, qml.variable.Variable):
-            params.update({k: v.val})
+    for k in diff_params:
+        # Since we cannot bind Variables to Qiskit circuits,
+        # we must remove them from the binding dictionary before binding.
+        del params[k]
 
     return quantum_circuit.bind_parameters(params)
 
@@ -83,7 +103,7 @@ def map_wires(wires: list, qc_wires: list) -> dict:
         qc_wires (list): wires from the converted quantum circuit
 
     Returns:
-        wire_map (dict): map from quantum circuit wires to the user defined wires
+        dict[int, int]: map from quantum circuit wires to the user defined wires
     """
     if wires is None:
         return dict(zip(qc_wires, range(len(qc_wires))))
@@ -105,17 +125,12 @@ def execute_supported_operation(operation_name: str, parameters: list, wires: li
     """
     operation = getattr(pennylane_ops, operation_name)
 
-    parameters = [_check_parameter_bound(param) for param in parameters]
-
     if not parameters:
         operation(wires=wires)
     elif operation_name == 'QubitStateVector':
         operation(np.array(parameters), wires=wires)
-    elif operation_name == 'QubitUnitary':
-        operation(*parameters, wires=wires)
     else:
-        float_params = [float(param) for param in parameters]
-        operation(*float_params, wires=wires)
+        operation(*parameters, wires=wires)
 
 
 def load(quantum_circuit: QuantumCircuit):
@@ -144,8 +159,8 @@ def _function(params: dict = None, wires: list = None):
         Returns:
             function: the new PennyLane template
         """
-
-        qc = _check_circuit_and_bind_parameters(quantum_circuit, params)
+        var_ref_map = _extract_variable_refs(params)
+        qc = _check_circuit_and_bind_parameters(quantum_circuit, params, var_ref_map)
 
         # Wires from a qiskit circuit are unique w.r.t. a register name and a qubit index
         qc_wires = [(q.register.name, q.index) for q in quantum_circuit.qubits]
@@ -160,8 +175,32 @@ def _function(params: dict = None, wires: list = None):
             operation_wires = [wire_map[(qubit.register.name, qubit.index)] for qubit in op[1]]
 
             if instruction_name in inv_map and inv_map[instruction_name] in pennylane_ops.ops:
-
-                execute_supported_operation(inv_map[instruction_name], op[0].params, operation_wires)
+                # Extract the bound parameters from the operation. If the bound parameters are a
+                # Qiskit ParameterExpression, then replace it with the corresponding PennyLane
+                # variable from the var_ref_map dictionary.
+
+                parameters = []
+                for p in op[0].params:
+
+                    _check_parameter_bound(p, var_ref_map)
+
+                    if isinstance(p, ParameterExpression):
+                        if p.parameters:
+                            # p.parameters must be a single parameter, as PennyLane
+                            # does not support expressions of variables currently.
+                            if len(p.parameters) > 1:
+                                raise ValueError("Operation {} has invalid parameter {}. PennyLane does not support "
+                                                 "expressions containing differentiable parameters as operation "
+                                                 "arguments".format(instruction_name, p))
+
+                            param = min(p.parameters)
+                            parameters.append(var_ref_map.get(param))
+                        else:
+                            parameters.append(float(p))
+                    else:
+                        parameters.append(p)
+
+                execute_supported_operation(inv_map[instruction_name], parameters, operation_wires)
 
             elif instruction_name == 'SdgGate':
 

diff --git a/requirements.txt b/requirements.txt
@@ -1,4 +1,4 @@
 qiskit>=0.15
-PennyLane>=0.7.0
+pennylane
 numpy
 networkx==2.3
diff --git a/tests/test_converter.py b/tests/test_converter.py
@@ -13,6 +13,11 @@
 from pennylane_qiskit.converter import load, load_qasm, load_qasm_from_file, map_wires
 
 
+THETA = np.linspace(0.11, 3, 5)
+PHI = np.linspace(0.32, 3, 5)
+VARPHI = np.linspace(0.02, 3, 5)
+
+
 class TestConverter:
     """Tests the converter function that allows converting QuantumCircuit objects
      to Pennylane templates."""
@@ -219,6 +224,21 @@ def test_parameter_was_not_bound(self, recorder):
             with recorder:
                 quantum_circuit(params={})
 
+    def test_invalid_parameter_expression(self, recorder):
+        """Tests that an operation with multiple parameters raises an error."""
+
+        theta = Parameter('θ')
+        phi = Parameter('φ')
+
+        qc = QuantumCircuit(3, 1)
+        qc.rz(theta*phi, [0])
+
+        quantum_circuit = load(qc)
+
+        with pytest.raises(ValueError, match='PennyLane does not support expressions'):
+            with recorder:
+                quantum_circuit(params={theta: qml.variable.Variable(0), phi: qml.variable.Variable(1)})
+
     def test_extra_parameters_were_passed(self, recorder):
         """Tests that loading raises an error when extra parameters were passed."""
 
@@ -551,7 +571,7 @@ def test_quantum_circuit_error_by_calling_wrong_parameters(self, recorder):
 
         quantum_circuit = load(qc)
 
-        with pytest.raises(ValueError, match="could not convert string to float: '{}'".format(angle)):
+        with pytest.raises(TypeError, match="parameter expected, got <class 'str'>"):
             with recorder:
                 quantum_circuit()
 
@@ -946,3 +966,37 @@ def circuit_native_pennylane():
             return qml.expval(qml.PauliZ(0))
 
         assert circuit_loaded_qiskit_circuit() == circuit_native_pennylane()
+
+    @pytest.mark.parametrize("analytic", [True])
+    @pytest.mark.parametrize("theta,phi,varphi", list(zip(THETA, PHI, VARPHI)))
+    def test_gradient(self, theta, phi, varphi, analytic, tol):
+        """Test that the gradient works correctly"""
+        qc = QuantumCircuit(3)
+        qiskit_params = [Parameter("param_{}".format(i)) for i in range(3)]
+
+        qc.rx(qiskit_params[0], 0)
+        qc.rx(qiskit_params[1], 1)
+        qc.rx(qiskit_params[2], 2)
+        qc.cnot(0, 1)
+        qc.cnot(1, 2)
+
+        # convert to a PennyLane circuit
+        qc_pl = qml.from_qiskit(qc)
+
+        dev = qml.device("default.qubit", wires=3, analytic=analytic)
+
+        @qml.qnode(dev)
+        def circuit(params):
+            qiskit_param_mapping = dict(map(list, zip(qiskit_params, params)))
+            qc_pl(qiskit_param_mapping)
+            return qml.expval(qml.PauliX(0) @ qml.PauliY(2))
+
+        dcircuit = qml.grad(circuit, 0)
+        res = dcircuit([theta, phi, varphi])
+        expected = [
+            np.cos(theta) * np.sin(phi) * np.sin(varphi),
+            np.sin(theta) * np.cos(phi) * np.sin(varphi),
+            np.sin(theta) * np.sin(phi) * np.cos(varphi)
+        ]
+
+        assert np.allclose(res, expected, **tol)