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

Updates for Qiskit to Braket circuit conversion #97

Merged
merged 7 commits into from
Jun 5, 2023
Merged
Changes from 3 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
7 changes: 4 additions & 3 deletions qiskit_braket_provider/providers/adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from typing import Callable, Dict, Iterable, List, Optional, Tuple, Union

from braket.aws import AwsDevice
from braket.circuits import Circuit, Instruction, gates, result_types
from braket.circuits import Circuit, Instruction, gates, result_types, Observable
from braket.device_schema import (
DeviceActionType,
GateModelQpuParadigmProperties,
Expand Down Expand Up @@ -347,11 +347,12 @@ def convert_qiskit_to_braket_circuit(circuit: QuantumCircuit) -> Circuit:
# TODO: change Probability result type for Sample for proper functioning # pylint:disable=fixme
# Getting the index from the bit mapping
quantum_circuit.add_result_type(
result_types.Probability(
result_types.Sample(
observable=Observable,
Copy link
Contributor

@dakk dakk Jun 1, 2023

Choose a reason for hiding this comment

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

Class Observable represents a quantum observable, but which one? Since Probability returns probabilities on computation basis, you have to put the computational basis observable

Copy link
Contributor Author

@urihan urihan Jun 1, 2023

Choose a reason for hiding this comment

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

Hi @dakk, Thank you for your comment. In my first commit, I used observable=Observable.Z() for the Probability; however, it was not passing the tox -elint test. I was changing codes to pass the tox -elint, which is why I got rid of the Z() part causing an error in tox -epy39.
So, I think you are right that I have to put a computational basis but not sure how to pass the tox -elint test..

Copy link
Contributor

Choose a reason for hiding this comment

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

What was the error of the linter?

Copy link
Contributor Author

@urihan urihan Jun 1, 2023

Choose a reason for hiding this comment

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

@dakk
qiskit_braket_provider/providers/adapter.py:351:31: E1101: Class 'Observable' has no 'Z' member (no-member)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wonder if we have to use StandardObservable class instead of just Observable class

Copy link
Contributor Author

@urihan urihan Jun 1, 2023

Choose a reason for hiding this comment

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

Right, seems like I have the same version. For that specific error, I added the comment # pylint:disable=fixme next to the code, and I think it might fix it.

But, now I am getting after running tox -elint

lint run-test: commands[3] | mypy --install-types --non-interactive .
error: --install-types failed (no mypy cache directory)

I tried to follow this instruction on mypy-install-types-mre, but only produces the same error. I might have to spend more time with it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe there is a problem with the linter that can't detect dynamically added members; observables are dynamically added to Observable class (from braket side).

There is a workaround:

from braket.circuits import observables
observables.Z()

If the problem is the linter, this should work.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks a lot for your help here @dakk!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wrote a comment here before, but I might have accidentally erased it. Thank you for your help @dakk. I found out I was still getting the same error even after updating with observables.Z(). I was able to get it to work after re-cloning my repository. I decided to just stick with the solution you provided though! I appreciate your help! :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I wrote a comment here before, but I might have accidentally erased it. Thank you for your help @dakk. I found out I was still getting the same error even after updating with observables.Z(). I was able to get it to work after re-cloning my repository. I decided to just stick with the solution you provided though! I appreciate your help! :)

Oh, that's weird! It was a pleasure!

target=[
circuit.find_bit(qiskit_gates[1][0]).index,
circuit.find_bit(qiskit_gates[2][0]).index,
]
],
)
)
elif name == "barrier":
Expand Down