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

Add GHZ circuit function #1499

Merged
merged 18 commits into from
Oct 26, 2024
Merged

Add GHZ circuit function #1499

merged 18 commits into from
Oct 26, 2024

Conversation

mho291
Copy link
Contributor

@mho291 mho291 commented Oct 22, 2024

Checklist:

  • Reviewers confirm new code works as expected.
  • Tests are passing.
  • Coverage does not decrease.
  • Documentation is updated.

Copy link

codecov bot commented Oct 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.93%. Comparing base (45eae7f) to head (bbb6da4).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1499   +/-   ##
=======================================
  Coverage   99.93%   99.93%           
=======================================
  Files          81       81           
  Lines       11907    11914    +7     
=======================================
+ Hits        11899    11906    +7     
  Misses          8        8           
Flag Coverage Δ
unittests 99.93% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mho291 mho291 marked this pull request as ready for review October 23, 2024 00:35
@renatomello renatomello self-requested a review October 23, 2024 05:14
@renatomello
Copy link
Contributor

@mho291 addition to api reference is missing

@renatomello renatomello self-requested a review October 23, 2024 06:41
@renatomello renatomello added this to the Qibo 0.2.14 milestone Oct 23, 2024
@MatteoRobbiati MatteoRobbiati self-requested a review October 23, 2024 09:20
@renatomello renatomello self-requested a review October 23, 2024 10:32


def ghz_state(nqubits: int, **kwargs):
"""Generates an :math:`n`-qubits Greenberger-Horne-Zeilinger (GHZ) state that takes the form
Copy link
Contributor

@renatomello renatomello Oct 24, 2024

Choose a reason for hiding this comment

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

Suggested change
"""Generates an :math:`n`-qubits Greenberger-Horne-Zeilinger (GHZ) state that takes the form
"""Generate a circuit that encodes a :math:`n`-qubit Greenberger-Horne-Zeilinger (GHZ) state.
This state that takes the form

Copy link
Contributor

Choose a reason for hiding this comment

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

@mho291 a tip: you can click the "Commit suggestion" button here to directly commit a suggestion to the code instead of changing things locally and then pushing it. It also helps to track which changes were already implemented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @renatomello ! I wanted to do extra checks for the documentation, so I did it locally and generated the html file. :)

@renatomello renatomello modified the milestones: Qibo 0.2.14, Qibo 0.2.13 Oct 24, 2024
@renatomello renatomello self-requested a review October 24, 2024 05:40
@renatomello renatomello changed the title Add GHZ circuit function Add GHZ circuit function Oct 24, 2024
Copy link
Contributor

@renatomello renatomello left a comment

Choose a reason for hiding this comment

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

@mho291 After you address my latest docstring comments, this is ready to go on my end.

@mho291
Copy link
Contributor Author

mho291 commented Oct 24, 2024

@mho291 After you address my latest docstring comments, this is ready to go on my end.

Thanks a lot @renatomello , I have modified the docstring accordingly.

@renatomello renatomello linked an issue Oct 25, 2024 that may be closed by this pull request
@scarrazza
Copy link
Member

@MatteoRobbiati could you please have another look?

Copy link
Contributor

@MatteoRobbiati MatteoRobbiati left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM.
A few (minor) suggestions follow.

Comment on lines +380 to +381
"""Generates an :math:`n`-qubit Greenberger-Horne-Zeilinger (GHZ) state that takes the form

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"""Generates an :math:`n`-qubit Greenberger-Horne-Zeilinger (GHZ) state that takes the form
"""Generate an :math:`n`-qubit Greenberger-Horne-Zeilinger (GHZ) state that takes the form

src/qibo/models/encodings.py Outdated Show resolved Hide resolved
@scarrazza scarrazza added this pull request to the merge queue Oct 26, 2024
Merged via the queue into master with commit 473bcf7 Oct 26, 2024
25 checks passed
@renatomello renatomello deleted the ghz branch October 28, 2024 05:20
@HuberyMing
Copy link

@mho291 Thank you for the addition of the GHZ circuit in models.encodings. I am going to use the added feature but found the following error:
ImportError: cannot import name 'ghz_state' from 'qibo.models.encodings' (/opt/anaconda3/envs/qiskit10/lib/python3.11/site-packages/qibo/models/encodings.py)

when I am importing it in my file head as

from qibo.models.encodings import (
comp_basis_encoder,
entangling_layer,
ghz_state,
phase_encoder,
unary_encoder,
unary_encoder_random_gaussian,
)

and it will cause no problem of importing if I am using

from qibo.models.encodings import (
comp_basis_encoder,
entangling_layer,
#ghz_state,
phase_encoder,
unary_encoder,
unary_encoder_random_gaussian,
)

How should I solve this problem? Thanks.

@renatomello
Copy link
Contributor

renatomello commented Oct 29, 2024 via email

@HuberyMing
Copy link

Are you using the newest version of qibo?
I have pulled the newest master branch and merge it into my_new_test_branch. Is that correct?

@HuberyMing
Copy link

Are you using the newest version of qibo?
I have pulled the newest master branch and merge it into my_new_test_branch. Is that correct?

Actually, even if I am in the newest master branch, it still does not work.

$ git pull origin master
From https://github.com/HuberyMing/qibo

  • branch master -> FETCH_HEAD
    Already up to date.

$ python test_noise.py
$ python test_models_encodings.py
Traceback (most recent call last):
File "/Users/mervyn/Library/CloudStorage/OneDrive-個人/Gsuite_NSYSU/4.QC_research/0.HH_work/B1.code/HHQC2024/qibo/tests/test_models_encodings.py", line 11, in
from qibo.models.encodings import (
ImportError: cannot import name 'ghz_state' from 'qibo.models.encodings' (/opt/anaconda3/envs/qiskit10/lib/python3.11/site-packages/qibo/models/encodings.py)

@renatomello
Copy link
Contributor

Are you using the newest version of qibo?
I have pulled the newest master branch and merge it into my_new_test_branch. Is that correct?

Actually, even if I am in the newest master branch, it still does not work.

$ git pull origin master From https://github.com/HuberyMing/qibo

* branch                master     -> FETCH_HEAD
  Already up to date.

$ python test_noise.py $ python test_models_encodings.py Traceback (most recent call last): File "/Users/mervyn/Library/CloudStorage/OneDrive-個人/Gsuite_NSYSU/4.QC_research/0.HH_work/B1.code/HHQC2024/qibo/tests/test_models_encodings.py", line 11, in from qibo.models.encodings import ( ImportError: cannot import name 'ghz_state' from 'qibo.models.encodings' (/opt/anaconda3/envs/qiskit10/lib/python3.11/site-packages/qibo/models/encodings.py)

I have the master installed locally and it works for me.

@HuberyMing
Copy link

Are you using the newest version of qibo?
I have pulled the newest master branch and merge it into my_new_test_branch. Is that correct?

Actually, even if I am in the newest master branch, it still does not work.
$ git pull origin master From https://github.com/HuberyMing/qibo

* branch                master     -> FETCH_HEAD
  Already up to date.

$ python test_noise.py $ python test_models_encodings.py Traceback (most recent call last): File "/Users/mervyn/Library/CloudStorage/OneDrive-個人/Gsuite_NSYSU/4.QC_research/0.HH_work/B1.code/HHQC2024/qibo/tests/test_models_encodings.py", line 11, in from qibo.models.encodings import ( ImportError: cannot import name 'ghz_state' from 'qibo.models.encodings' (/opt/anaconda3/envs/qiskit10/lib/python3.11/site-packages/qibo/models/encodings.py)

I have the master installed locally and it works for me.

Okay. Currently I am using qibo 0.2.12. I will create a new environment and see if it works later. Thanks.

Python 3.11.9 (main, Apr 19 2024, 11:43:47) [Clang 14.0.6 ] on darwin
Type "help", "copyright", "credits" or "license" for more information.

import qibo
qibo.version
'0.2.12'

@HuberyMing
Copy link

Are you using the newest version of qibo?
I have pulled the newest master branch and merge it into my_new_test_branch. Is that correct?

Actually, even if I am in the newest master branch, it still does not work.
$ git pull origin master From https://github.com/HuberyMing/qibo

* branch                master     -> FETCH_HEAD
  Already up to date.

$ python test_noise.py $ python test_models_encodings.py Traceback (most recent call last): File "/Users/mervyn/Library/CloudStorage/OneDrive-個人/Gsuite_NSYSU/4.QC_research/0.HH_work/B1.code/HHQC2024/qibo/tests/test_models_encodings.py", line 11, in from qibo.models.encodings import ( ImportError: cannot import name 'ghz_state' from 'qibo.models.encodings' (/opt/anaconda3/envs/qiskit10/lib/python3.11/site-packages/qibo/models/encodings.py)

I have the master installed locally and it works for me.

Okay. Currently I am using qibo 0.2.12. I will create a new environment and see if it works later. Thanks.

Python 3.11.9 (main, Apr 19 2024, 11:43:47) [Clang 14.0.6 ] on darwin Type "help", "copyright", "credits" or "license" for more information.

import qibo
qibo.version
'0.2.12'

It works when I created a whole new environment. Probably that is because of the qibo version as you said. The qibo 0.2.13 works for it now.

@renatomello
Copy link
Contributor

It works when I created a whole new environment. Probably that is because of the qibo version as you said. The qibo 0.2.13 works for it now.

Release 0.2.12 was released prior to the GHZ PR. So now the ghz function is in release 0.2.13 as well as the master branch.

@HuberyMing
Copy link

It works when I created a whole new environment. Probably that is because of the qibo version as you said. The qibo 0.2.13 works for it now.

Release 0.2.12 was released prior to the GHZ PR. So now the ghz function is in release 0.2.13 as well as the master branch.

I see. Thank you for the explanation.

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.

Add GHZ state to models.encodings
5 participants