Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed bug in gradients of loaded qiskit circuits #71

Merged
merged 10 commits into from
Feb 28, 2020
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
87 changes: 63 additions & 24 deletions pennylane_qiskit/converter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.VariableRef]):
"""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.VariableRef]):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point, it might be worth considering the typing module:

from typing import Dict

def _check_parameter_bound(param: ..., var_ref_map: Dict[Parameter, VariableRef]) -> Parameter:
    """
    ...
    var_ref_map (Dict[Parameter, VariableRef]): ...
    """
    ...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, I will add that in!

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.VariableRef]:
"""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.VariableRef]: 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 VariableRefs.
if params is None:
return {}

return {k: v for k, v in params.items() if isinstance(v, qml.variable.VariableRef)}


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
VariableRef 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 VariableRefs to Qiskit circuits,
# we must remove them from the binding dictionary before binding.
del params[k]

return quantum_circuit.bind_parameters(params)

Expand All @@ -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))))
Expand All @@ -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):
Expand Down Expand Up @@ -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]
Expand All @@ -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)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't help but feel we can use the return value of _check_parameter_bound

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed the return statement on _check_parameter_bound, since it is never used


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':

Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
qiskit>=0.15
PennyLane>=0.7.0
git+git://github.com/XanaduAI/pennylane.git#egg=pennylane

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why this is needed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A recent PR in PennyLane master renamed the Variable class to VariableRef --- I wanted to double check that this PR passes against the latest master of PennyLane. However, this renaming will likely be reverted due to side effects that occurred, so this change here is temporary and should be reverted before this PR is merged.

numpy
networkx==2.3
56 changes: 55 additions & 1 deletion tests/test_converter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down Expand Up @@ -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.VariableRef(0), phi: qml.variable.VariableRef(1)})

def test_extra_parameters_were_passed(self, recorder):
"""Tests that loading raises an error when extra parameters were passed."""

Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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)