Skip to content

Changed EfficientSU2 to efficient_su2 #2841

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

Merged
merged 20 commits into from
Mar 26, 2025
Merged

Changed EfficientSU2 to efficient_su2 #2841

merged 20 commits into from
Mar 26, 2025

Conversation

kaldag
Copy link
Contributor

@kaldag kaldag commented Mar 24, 2025

No description provided.

@qiskit-bot
Copy link
Contributor

Thanks for contributing to Qiskit documentation!

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. Thanks! 🙌

One or more of the following people are relevant to this code:

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@CLAassistant
Copy link

CLAassistant commented Mar 24, 2025

CLA assistant check
All committers have signed the CLA.

@kaldag
Copy link
Contributor Author

kaldag commented Mar 24, 2025

Fixes #2811
Fixes Qiskit/qiskit#14075

Copy link
Member

@ShellyGarion ShellyGarion left a comment

Choose a reason for hiding this comment

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

Hi @kaldag. thanks for your contribution.
My main comment is that you should try to locally run the code to see that it works.
for efficient_su2 the decompose() is no longer needed and there is no flatten parameter.

from qiskit_ibm_transpiler.ai.routing import AIRouting

ai_passmanager = PassManager([
AIRouting(backend_name="ibm_sherbrooke", optimization_level=2, layout_mode="optimize", local_mode=True)
])

circuit = EfficientSU2(101, entanglement="circular", reps=1).decompose()
circuit = efficient_su2(101, entanglement="circular", reps=1).decompose()
Copy link
Member

Choose a reason for hiding this comment

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

decompose is not needed here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected.

@@ -84,7 +84,7 @@ ai_passmanager = PassManager([
AIPauliNetworkSynthesis(backend_name="ibm_cairo", local_mode=False), # Re-synthesize Pauli Network blocks. Only available in the Qiskit Transpiler Service
])

circuit = EfficientSU2(10, entanglement="full", reps=1).decompose()
circuit = efficient_su2(10, entanglement="full", reps=1).decompose()
Copy link
Member

Choose a reason for hiding this comment

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

decompose is not needed here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected.

from qiskit_ibm_runtime import QiskitRuntimeService

backend = QiskitRuntimeService().backend("ibm_fez")
fez_coupling_map = backend.coupling_map

su2_circuit = EfficientSU2(101, entanglement="circular", reps=1).decompose()
su2_circuit = efficient_su2(101, entanglement="circular", reps=1).decompose()
Copy link
Member

Choose a reason for hiding this comment

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

decompose is not needed here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected.

"\n",
"circuit = EfficientSU2(127, entanglement=\"linear\", flatten=True)\n",
"circuit = efficient_su2(127, entanglement=\"linear\", flatten=True)\n",
Copy link
Member

Choose a reason for hiding this comment

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

remove the flatten argument which does not exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected.

from qiskit_ibm_transpiler.transpiler_service import TranspilerService

circuit = EfficientSU2(101, entanglement="circular", reps=1).decompose()
circuit = efficient_su2(101, entanglement="circular", reps=1).decompose()
Copy link
Member

Choose a reason for hiding this comment

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

decompose is not needed.

from qiskit_ibm_transpiler.transpiler_service import TranspilerService

circuit = EfficientSU2(101, entanglement="circular", reps=1).decompose()
circuit = efficient_su2(101, entanglement="circular", reps=1).decompose()
Copy link
Member

Choose a reason for hiding this comment

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

decompose is not needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected.

from qiskit_ibm_transpiler.transpiler_service import TranspilerService

circuit = EfficientSU2(101, entanglement="circular", reps=1).decompose()
circuit = efficient_su2(101, entanglement="circular", reps=1).decompose()
Copy link
Member

Choose a reason for hiding this comment

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

decompose is not needed.

@@ -47,7 +47,7 @@
},
{
"cell_type": "code",
"execution_count": 1,
"execution_count": null,
"id": "df70b5fd-971d-4e7d-a23a-8df037c0fa47",
Copy link
Member

Choose a reason for hiding this comment

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

why is this change needed here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was a mistake. Corrected.

"\n",
"qc = EfficientSU2(12, entanglement=\"circular\", reps=1)\n",
"qc = efficient_su2(12, entanglement=\"circular\", reps=1)\n",
"\n",
"qc.decompose().draw(\"mpl\")"
Copy link
Member

Choose a reason for hiding this comment

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

decompose is not needed here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected.

"\n",
"n_qubits = 8\n",
"circuit = EfficientSU2(n_qubits)\n",
"circuit = efficient_su2(n_qubits)\n",
"circuit.decompose().draw(\"mpl\")"
Copy link
Member

Choose a reason for hiding this comment

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

decompose is not needed here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected.

"\n",
"n_qubits = 8\n",
"circuit = EfficientSU2(n_qubits)\n",
"circuit = efficient_su2(n_qubits)\n",
"circuit.decompose().draw(\"mpl\")"
Copy link
Member

Choose a reason for hiding this comment

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

decompose is not needed here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected.

@@ -180,7 +180,7 @@
"\n",
"where $n$ is the number of qubits, in this case, 2. That is, with probability $p$, the state is replaced with the completely mixed state, and the state is preserved with probability $1 - p$. After $m$ applications of the depolarizing channel, the probability of the state being preserved would be $(1 - p)^m$. Therefore, we expect the probability of retaining the correct state at the end of the simulation to go down exponentially with the number of CX gates in our circuit.\n",
"\n",
"Let's count the number of CX gates in our circuit and compute $(1 - p)^m$. Because our circuit uses the EfficientSU2 class, we'll need to call `decompose` once to decompose it into CX gates. We call `count_ops` to get a dictionary that maps gate names to counts, and retrieve the entry for the CX gate."
"Let's count the number of CX gates in our circuit and compute $(1 - p)^m$. Because our circuit uses the efficient_su2 class, we'll need to call `decompose` once to decompose it into CX gates. We call `count_ops` to get a dictionary that maps gate names to counts, and retrieve the entry for the CX gate."
Copy link
Member

Choose a reason for hiding this comment

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

no need to call decompose anymore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected.

@kaldag
Copy link
Contributor Author

kaldag commented Mar 24, 2025

Hi @kaldag. thanks for your contribution. My main comment is that you should try to locally run the code to see that it works. for efficient_su2 the decompose() is no longer needed and there is no flatten parameter.

Hi @ShellyGarion. Thank you for the response. I did try locally. I did not get error with decompose(), hence did not tamper with it. However, the 'flatten' parameter was overlooked, as you correctly pointed out. I have corrected all of them.

@ShellyGarion
Copy link
Member

ShellyGarion commented Mar 25, 2025

LGTM. I don't understand why the CI tests are failing

@ShellyGarion ShellyGarion self-requested a review March 25, 2025 07:53
@frankharkins
Copy link
Member

Thanks! The CI failure is not your fault, it's because we can't run that test on pull requests forks.

Copy link
Member

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

@frankharkins
Copy link
Member

@kaldag thank you for making this PR and for signing the CLA. Could you please also link your committer email to your GitHub account so the CLA check passes? See instructions: #2841 (comment)

@ShellyGarion
Copy link
Member

Hi @kaldag - thanks again for this PR, I already handled all the other notebooks in #2852 and #2848.
could you please handle the CLA so that we can merge this PR? thanks!

kaldag and others added 7 commits March 25, 2025 23:24
An action recently synced the latest dev docs. This PR updates all dev
APIs that changed.
  > [!NOTE]
  > This pull request was created by a GitHub action.

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This PR makes three changes to the API generation script:

- **Recognize C `typedef` and format as class**

- **Do not use dedicated pages in C API**
  
In Python APIs, if a class has the same name as the page name, the page
will be a "dedicated page" for that class. This commit disables that for
the C API. The C API pages are a grouping of structs and associated
functions, rather than a class and its methods. This change reflects
that in the information heirarchy.

- **Fix TOC for C API**

C does not have modules, so we can't group pages by module. This commit
just lists all C API pages in the TOC.
Regenerates the C API from a new artifact generated from
https://github.com/frankharkins/qiskit-terra/tree/FH/c-api-docs. I did
not check in images from the release notes or the `objects.inv` files as
they'll eventually be removed during the generation script.
@kaldag
Copy link
Contributor Author

kaldag commented Mar 25, 2025

Hi @kaldag - thanks again for this PR, I already handled all the other notebooks in #2852 and #2848. could you please handle the CLA so that we can merge this PR? thanks!

Thank you. I have amended the author and email.

@Eric-Arellano
Copy link
Collaborator

Thanks, @kaldag, for signing the CLA!

It looks like this PR now has some merge conflicts. Feel free to either rebase or do a normal merge with main.

execution count had to changed in cell.
kaldag added 2 commits March 25, 2025 23:53
Execution count had to changed in cell code.
cell execution was changed.
@Eric-Arellano
Copy link
Collaborator

It looks like this PR now has some merge conflicts. Feel free to either rebase or do a normal merge with main.

The easiest approach might be an interactive rebase.

git fetch --all
git rebase -i upstream/main

In the rebase, you'll want to make sure the following commits don't show up because they are not relevant to your change and they are causing merge conflicts. This guide may be helpful: https://docs.github.com/en/get-started/using-git/about-git-rebase. Or there are lots of other good online tutorials on interactive rebases.

Screenshot 2025-03-25 at 2 29 22 PM

@Eric-Arellano
Copy link
Collaborator

Thanks! @ShellyGarion @Cryoris can you please re-confirm that this PR looks good to double check that nothing was broken when dealing with the merge conflict? We will merge this after getting your confirmation.

Copy link
Member

@ShellyGarion ShellyGarion left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!
Looks good except one broken link, perhaps you can merge it now and I'll fix it in #2852?

@@ -80,8 +80,7 @@
"id": "4064b5ad-8a3a-462b-ae30-29576230c084",
"metadata": {},
"source": [
"The example circuit uses an instance of [`EfficientSU2`](/api/qiskit/qiskit.circuit.library.EfficientSU2#efficientsu2) from Qiskit's circuit library.\n",
"The following cell decomposes the circuit before drawing it to show its gate structure."
"The example circuit uses an instance of [`efficient_su2`](/api/qiskit/qiskit.circuit.library.EfficientSU2#efficientsu2) from Qiskit's circuit library."
Copy link
Member

Choose a reason for hiding this comment

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

the link should be api/qiskit/qiskit.circuit.library.efficient_su2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The link has been changed.

Copy link
Member

Choose a reason for hiding this comment

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

thanks, but it should be:
api/qiskit/qiskit.circuit.library.efficient_su2
and not:
api/qiskit/qiskit.circuit.library.efficientsu2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed.

kaldag added 2 commits March 26, 2025 13:58
link changed to /api/qiskit/qiskit.circuit.library.efficient_su2
@ElePT ElePT enabled auto-merge March 26, 2025 09:05
@ElePT ElePT added this pull request to the merge queue Mar 26, 2025
Merged via the queue into Qiskit:main with commit 78eaadc Mar 26, 2025
3 of 4 checks passed
frankharkins added a commit that referenced this pull request Apr 1, 2025
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Frank Harkins <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants