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

Conversation

urihan
Copy link
Contributor

@urihan urihan commented May 29, 2023

Summary

resolves #37 by changing Probability result type to Sample result type.

Details and comments

I have added an additional class, Observable, to the adapter.py file. Also updated the Sampler result type itself by adding an observable z as a parameter.

Copy link
Collaborator

@kshitijc kshitijc left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution @urihan! 🚀 Could you please make changes to:

  • Fix the failing checks
  • Add unit tests for the source code changes

@urihan
Copy link
Contributor Author

urihan commented May 31, 2023

Hi @kshitijc, I am still working on this! I've been stuck on this error called Class 'Observable' has no 'Z' member (no-member) when running 'tox -elint' but it passes 'tox -epy39'.
It is my first time contributing, so I am little lost with this error. Do you have a suggestion on fixing this error? Thank you!

@kshitijc
Copy link
Collaborator

Hi @kshitijc, I am still working on this! I've been stuck on this error called Class 'Observable' has no 'Z' member (no-member) when running 'tox -elint' but it passes 'tox -epy39'. It is my first time contributing, so I am little lost with this error. Do you have a suggestion on fixing this error? Thank you!

Sorry, I don't see Observable.Z() in your file diff. Could you please point me to the exact location where you are encountering this error?

@@ -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!

@urihan
Copy link
Contributor Author

urihan commented Jun 5, 2023

Hi @kshitijc, I have finally fixed failing checks and also added a unit test! It would be great if you could check my submission. Thank you!

Copy link
Collaborator

@kshitijc kshitijc left a comment

Choose a reason for hiding this comment

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

Great job @urihan! 👍

@kshitijc kshitijc merged commit 5fef143 into qiskit-community:main Jun 5, 2023
robotAstray pushed a commit to robotAstray/qiskit-braket-provider that referenced this pull request Jun 14, 2023
…y#97)

* Probability to Sample result type

* passes tox lint

* updates toxlint

* fixes observable z error but not passses elint

* unit test added

* final commit with tests

fix: Fallback to JAQCD for device properties (qiskit-community#104)

* fix: Fallback to JAQCD for device properties

* fix: reformat

replacing AWSBraketJob with AmazonBraketTask

run the notebooks containing AmazonBraketTask

deprecation warning added for AWSBraketJob class

formatting fixed

re-add <JOB_ARN>

fix breaking changes on /provider/__init__.py and __init__.py

rename job_id to task_id argument for AmazonBraketTask()

rename braket_jobs_states to braket_tasks_states

add task_id method to access the task_id similar to the one we have in the JobV1 class

adding tests for new class 'AmazonBraketTask' and old class 'AWSBraketJob'

formatting test_braket_job.py

correcting definition of AWSBraketJob

rewriting line 184 in braket_job.py

adding class docstring to AWSBraketJob

standard import 'from warnings import warn'  placed before 'from braket.aws import AwsQuantumTask' (wrong-import-order)

docstring fix in test_braket_job.py

assert the job id in test_AWS_job() function in test_braket_job.py

running how to notebook #0

running how to notebook #1

running how to notebook #2

running how to notebook #3

running how to notebook number 5

running tutorial  notebook number 3

tutorial number 0
christianbmadsen pushed a commit that referenced this pull request Jun 19, 2023
* fix: Updates for Qiskit to Braket circuit conversion (#97)

* Probability to Sample result type

* passes tox lint

* updates toxlint

* fixes observable z error but not passses elint

* unit test added

* final commit with tests

fix: Fallback to JAQCD for device properties (#104)

* fix: Fallback to JAQCD for device properties

* fix: reformat

replacing AWSBraketJob with AmazonBraketTask

run the notebooks containing AmazonBraketTask

deprecation warning added for AWSBraketJob class

formatting fixed

re-add <JOB_ARN>

fix breaking changes on /provider/__init__.py and __init__.py

rename job_id to task_id argument for AmazonBraketTask()

rename braket_jobs_states to braket_tasks_states

add task_id method to access the task_id similar to the one we have in the JobV1 class

adding tests for new class 'AmazonBraketTask' and old class 'AWSBraketJob'

formatting test_braket_job.py

correcting definition of AWSBraketJob

rewriting line 184 in braket_job.py

adding class docstring to AWSBraketJob

standard import 'from warnings import warn'  placed before 'from braket.aws import AwsQuantumTask' (wrong-import-order)

docstring fix in test_braket_job.py

assert the job id in test_AWS_job() function in test_braket_job.py

running how to notebook #0

running how to notebook #1

running how to notebook #2

running how to notebook #3

running how to notebook number 5

running tutorial  notebook number 3

tutorial number 0

* add separate QUEUED and RUNNING states to task status (#46)

---------

Co-authored-by: Yuri Han <45699207+urihan@users.noreply.github.com>
robotAstray added a commit to robotAstray/qiskit-braket-provider that referenced this pull request Jun 19, 2023
* [UnitaryHack] Enable qiskit transpilation discontinous qubit indices (qiskit-community#108)

* convert Rigetti Aspen qubit connectivity continous

* Add qiskit transpilation discountinous qubit indices

* Add more comments, cleanup

* Add more comments, cleanup

* Move in Rigetti branching since not require globally

* Fix lint

* Add better example

* Fix docstring

* Add mock for rigetti aspen m3

* Enable transpilation test

* Fix mypy

* [UnitaryHack] Gate Decomposition(qiskit-community#90) added and tested (qiskit-community#111)

* implement and test gate decomposition

* Implement and Test Gate Decomposition

* addressed comments in previous PR

* Adding separate QUEUED and RUNNING states to task status (qiskit-community#110)

* fix: Updates for Qiskit to Braket circuit conversion (qiskit-community#97)

* Probability to Sample result type

* passes tox lint

* updates toxlint

* fixes observable z error but not passses elint

* unit test added

* final commit with tests

fix: Fallback to JAQCD for device properties (qiskit-community#104)

* fix: Fallback to JAQCD for device properties

* fix: reformat

replacing AWSBraketJob with AmazonBraketTask

run the notebooks containing AmazonBraketTask

deprecation warning added for AWSBraketJob class

formatting fixed

re-add <JOB_ARN>

fix breaking changes on /provider/__init__.py and __init__.py

rename job_id to task_id argument for AmazonBraketTask()

rename braket_jobs_states to braket_tasks_states

add task_id method to access the task_id similar to the one we have in the JobV1 class

adding tests for new class 'AmazonBraketTask' and old class 'AWSBraketJob'

formatting test_braket_job.py

correcting definition of AWSBraketJob

rewriting line 184 in braket_job.py

adding class docstring to AWSBraketJob

standard import 'from warnings import warn'  placed before 'from braket.aws import AwsQuantumTask' (wrong-import-order)

docstring fix in test_braket_job.py

assert the job id in test_AWS_job() function in test_braket_job.py

running how to notebook #0

running how to notebook #1

running how to notebook #2

running how to notebook #3

running how to notebook number 5

running tutorial  notebook number 3

tutorial number 0

* add separate QUEUED and RUNNING states to task status (qiskit-community#46)

---------

Co-authored-by: Yuri Han <45699207+urihan@users.noreply.github.com>

---------

Co-authored-by: WingCode <smallstar1234@gmail.com>
Co-authored-by: Gmontes01 <118773601+Gmontes01@users.noreply.github.com>
Co-authored-by: Yuri Han <45699207+urihan@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Updates for Qiskit to Braket circuit conversion
3 participants