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.

Sign up for GitHub

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

Jump to bottom

Properly substitute symbolic parameters #152

Merged
merged 3 commits into from
Feb 12, 2024
Merged

Properly substitute symbolic parameters #152

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
39c07fb
Properly substitute symbolic parameters
speller26 Feb 11, 2024
b53810c
Update qiskit_braket_provider/providers/adapter.py
speller26 Feb 11, 2024
fd612d2
Update tests/providers/test_adapter.py
speller26 Feb 12, 2024
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
15 changes: 6 additions & 9 deletions qiskit_braket_provider/providers/adapter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -521,18 +521,15 @@ def _create_qiskit_gate(
gate_name: str, gate_params: list[Union[float, Parameter]]
) -> Instruction:
gate_instance = _GATE_NAME_TO_QISKIT_GATE.get(gate_name, None)
if gate_instance is None:
raise TypeError(f'Braket gate "{gate_name}" not supported in Qiskit')
gate_cls = gate_instance.__class__
new_gate_params = []
for param_expression, value in zip(gate_instance.params, gate_params):
# Assumes that each Qiksit gate has one free parameter per expression
speller26 marked this conversation as resolved.
Show resolved Hide resolved
param = list(param_expression.parameters)[0]
if isinstance(value, Parameter):
new_gate_params.append(value)
else:
bound_param_expression = param_expression.bind({param: value})
new_gate_params.append(bound_param_expression)
if gate_instance is not None:
gate_cls = gate_instance.__class__
else:
raise TypeError(f'Braket gate "{gate_name}" not supported in Qiskit')
assigned = param_expression.assign(param, value)
Copy link
Collaborator

Choose a reason for hiding this comment

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

assign is the function I was looking for. So glad you bring this up. This is much better that the fix I was implementing on my side.
Side-effect, it works when passing a ParameterExpression

new_gate_params.append(assigned)
return gate_cls(*new_gate_params)


Expand Down
117 changes: 49 additions & 68 deletions tests/providers/test_adapter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,79 +503,60 @@ def test_type_error_on_bad_input(self):

def test_all_standard_gates(self):
"""
Tests braket to qiskit conversion with standard gates.
"""

gate_set = [attr for attr in dir(Gate) if attr[0].isupper()]

for gate_name in gate_set:
if (
gate_name.lower() not in _GATE_NAME_TO_QISKIT_GATE
or gate_name.lower() in ["gpi", "gpi2", "ms"]
):
continue

gate = getattr(Gate, gate_name)
if issubclass(gate, AngledGate):
op = gate(0.1)
elif issubclass(gate, TripleAngledGate):
op = gate(0.1, 0.1, 0.1)
else:
op = gate()
target = range(op.qubit_count)
instr = Instruction(op, target)

braket_circuit = Circuit().add_instruction(instr)
qiskit_circuit = to_qiskit(braket_circuit)

expected_qiskit_circuit = QuantumCircuit(op.qubit_count)
expected_qiskit_circuit.append(
_GATE_NAME_TO_QISKIT_GATE.get(gate_name.lower()), target
)
expected_qiskit_circuit.measure_all()
expected_qiskit_circuit = expected_qiskit_circuit.assign_parameters(
{p: 0.1 for p in expected_qiskit_circuit.parameters}
)

self.assertEqual(qiskit_circuit, expected_qiskit_circuit)

def test_all_ionq_gates(self):
Tests Braket to Qiskit conversion with standard gates.
"""
Tests braket to qiskit conversion with ionq gates.

Note: Braket gate angles are in radians while Qiskit IonQ gates expect turns.
"""
gate_set = [
attr
for attr in dir(Gate)
if attr[0].isupper() and attr.lower() in _GATE_NAME_TO_QISKIT_GATE
]

gate_set = ["GPi", "GPi2", "MS"]
# pytest.mark.parametrize is incompatible with TestCase
param_sets = [
[0.1, 0.2, 0.3],
[FreeParameter("a"), FreeParameter("b"), FreeParameter("c")],
]

for gate_name in gate_set:
gate = getattr(Gate, gate_name)
value = 0.1
qiskit_gate_cls = _GATE_NAME_TO_QISKIT_GATE.get(gate_name.lower()).__class__
qiskit_value = 0.1 / (2 * np.pi)
if issubclass(gate, AngledGate):
op = gate(value)
qiskit_gate = qiskit_gate_cls(qiskit_value)
elif issubclass(gate, TripleAngledGate):
args = [value] * 3
qiskit_args = [qiskit_value] * 3
op = gate(*args)
qiskit_gate = qiskit_gate_cls(*qiskit_args)
else:
op = gate()
qiskit_gate = qiskit_gate_cls()

target = range(op.qubit_count)
instr = Instruction(op, target)

braket_circuit = Circuit().add_instruction(instr)
qiskit_circuit = to_qiskit(braket_circuit)

expected_qiskit_circuit = QuantumCircuit(op.qubit_count)
expected_qiskit_circuit.append(qiskit_gate, target)
expected_qiskit_circuit.measure_all()

self.assertEqual(qiskit_circuit, expected_qiskit_circuit)
for params_braket in param_sets:
gate = getattr(Gate, gate_name)
if issubclass(gate, AngledGate):
op = gate(params_braket[0])
elif issubclass(gate, TripleAngledGate):
op = gate(*params_braket)
else:
op = gate()
target = range(op.qubit_count)
instr = Instruction(op, target)

braket_circuit = Circuit().add_instruction(instr)
qiskit_circuit = to_qiskit(braket_circuit)
param_uuids = {
param.name: param._uuid for param in qiskit_circuit.parameters
}
params_qiskit = [
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not just passing qiskit_circuit.parameters? We could assert that qiskit.parameters names are the same as those in params if we want to check the parameters.

Parameter(param.name, uuid=param_uuids.get(param.name))
if isinstance(param, FreeParameter)
else param
for param in params_braket
]

expected_qiskit_circuit = QuantumCircuit(op.qubit_count)
qiskit_gate = _GATE_NAME_TO_QISKIT_GATE.get(gate_name.lower())
expected_qiskit_circuit.append(qiskit_gate, target)
Copy link
Collaborator

Choose a reason for hiding this comment

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

for IonQ, I think this test would still miss the 2pi scaling factor since we are using directly the gate from the dict. We could maybe try to test the unitaries or a shot=0 sim.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That would make sense; the ideal way would be to compare the parametrized unitaries.

expected_qiskit_circuit.measure_all()
expected_qiskit_circuit = expected_qiskit_circuit.assign_parameters(
dict(
zip(
# Need to use gate parameters because
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would this be simpler if we use qiskit_circuit.parameters?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The problem is qiskit_circuit.parameters orders the parameters alphabetically, and the parameters in the _GATE_NAME_TO_BRAKET_GATE translations aren't. For example, the order of u parameters is theta, phi, lambda, but the alphabetical order is theta, lambda, phi.

# circuit parameters are sorted alphabetically
speller26 marked this conversation as resolved.
Show resolved Hide resolved
[list(expr.parameters)[0] for expr in qiskit_gate.params],
params_qiskit[: len(qiskit_gate.params)],
)
)
)
self.assertEqual(qiskit_circuit, expected_qiskit_circuit)

def test_parametric_gates(self):
"""
Expand Down
Loading