Skip to content

Conversation

ohadlev77
Copy link

Summary

This PR fixes issue #9080 by adding 2 examples for using the NLocal class.

Details and comments

I have added 2 examples for using the NLocal class that emphasize the necessary aspects.

Example 1 uses pre-defined QuantumCircuit objects for the rotation and entanglement layers.

Example 2 demonstrates the specific interface of the NLocal class compared to other parameterized circuit classes (e.g TwoLocal or EfficiensSU2) - one needs to provide the actual gate objects for the rotation_blocks / entanglement_blocks arguments (e.g RYGate(), RXGate(), etc). In the case of the other parameterized circuit classes, one can specify a specific string for a desired gate object (e.g 'ry' for an RY gate).

Following the convention in the docstrings of the other parameterized circuit classes, I didn't include any import statements within the examples. Of course, it can be done if we desire this, but I preferred to stick with the current convention that I've observed.

@ohadlev77 ohadlev77 requested a review from a team as a code owner December 18, 2022 10:06
@qiskit-bot qiskit-bot added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Dec 18, 2022
@qiskit-bot
Copy link
Collaborator

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the the following people are requested to review this:

@ohadlev77
Copy link
Author

@Cryoris @ajavadia @1ucian0

There is a failure in the tests for python3.11 but I think it has nothing to do with my commit.
Can one of you confirm? Should I do something?

@HuangJunye
Copy link
Collaborator

@ohadlev77 I think you are right. The failing Python 3.11 is not due to your code. See #9296

@HuangJunye HuangJunye self-assigned this Dec 19, 2022
Copy link
Collaborator

@HuangJunye HuangJunye left a comment

Choose a reason for hiding this comment

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

Can you please put the code inside jupyter-execute directives so that the code can be executed and tested during the building process?

@HuangJunye HuangJunye added documentation Something is not clear or an error documentation Changelog: None Do not include in changelog labels Dec 23, 2022
@ohadlev77
Copy link
Author

@HuangJunye
Done.

@ohadlev77
Copy link
Author

@HuangJunye can you please check the PR?

@HuangJunye
Copy link
Collaborator

@ohadlev77 As discussed via Slack, we have removed the usage of jupyter-execute directive in the documentation, can you please update the code examples using testcode and testoutput directives instead? You can find the instructions here: https://qiskit.github.io/qiskit_sphinx_theme/sphinx_guide/how_to_add_code.html#sphinx-testcode-and-sphinx-testoutput. Please reach out to me if you are unsure about how to do that.

Comment on lines 68 to 74
# Example 1:
# "Blocks" are defined by `QuantumCircuit` objects.
# The "basic rotation block" consists of a 1-depth 2-qubits `QuantumCircuit` object.
# The "basic entanglement block" consists of a 3-depth 4-qubits `QuantumCircuit` object.
# Every "rotation layer" consists of a vertical repetition over the basic rotation block.
# Every "entanglement layer" consists of a `linear` repetition over
# the basic entanglement block.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you could move this explanation outside of the code block, then it would be rendered as normal text and be easier to read 🙂

Copy link
Author

Choose a reason for hiding this comment

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

Sure

Comment on lines 118 to 120
# In the following `***_blocks` arguments is necessary to specify the actual gate objects,
# rather than some abbreviations that are used in other classes,
# e.g `CZGate()` = right, `cz` = wrong.
Copy link
Contributor

Choose a reason for hiding this comment

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

If users are not familiar with specification per string, I think this is more confusing than clarifying. How about just removing that subsentence? 🙂

Suggested change
# In the following `***_blocks` arguments is necessary to specify the actual gate objects,
# rather than some abbreviations that are used in other classes,
# e.g `CZGate()` = right, `cz` = wrong.
# In the following `***_blocks` arguments is necessary to specify the Gate objects

Copy link
Author

Choose a reason for hiding this comment

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

Sounds OK, Removing.

@1ucian0
Copy link
Member

1ucian0 commented Jun 27, 2023

Hello @ohadlev77 ! are you still working on this PR?

@1ucian0 1ucian0 assigned Cryoris and unassigned HuangJunye Jun 27, 2023
@ohadlev77
Copy link
Author

@1ucian0 - yes.
@Cryoris - fixed issues. I didn't change the jupyter-execute blocks because otherwise circuit-plots will not appear. Do we prefer to change it to a static circuit text-diagrams?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog Community PR PRs from contributors that are not 'members' of the Qiskit repo documentation Something is not clear or an error documentation
Projects
Status: Contributor MIA
Status: No status
Development

Successfully merging this pull request may close these issues.

5 participants