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

2274 Benchmark User Guide #2566

Merged
merged 34 commits into from
Dec 12, 2024

Conversation

Shivansh20128
Copy link
Contributor

Description

This PR adds a user guide section for benchmarks.
Closes issue #2274


License

  • I license this contribution under the terms of the GNU GPL, version 3 and grant Unitary Fund the right to provide additional permissions as described in section 7 of the GNU GPL, version 3.

Before opening the PR, please ensure you have completed the following where appropriate.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hello @Shivansh20128, thank you for submitting a PR to Mitiq! We will respond as soon as possible, and if you have any questions in the meantime, you can ask us on the Unitary Fund Discord.

@Shivansh20128
Copy link
Contributor Author

Hi! Can someone review this draft PR and reply if this is the kind of documentation the project needs? I will then add the rest of it accordingly.
Thank You

Copy link

codecov bot commented Nov 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.73%. Comparing base (3315184) to head (3701a61).
Report is 15 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2566   +/-   ##
=======================================
  Coverage   98.72%   98.73%           
=======================================
  Files          92       92           
  Lines        4168     4176    +8     
=======================================
+ Hits         4115     4123    +8     
  Misses         53       53           

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

@natestemen natestemen linked an issue Nov 12, 2024 that may be closed by this pull request
@natestemen
Copy link
Member

@Shivansh20128 this is exactly the kind of thing we're looking for!

@Shivansh20128
Copy link
Contributor Author

Shivansh20128 commented Nov 12, 2024

@purva-thakre Can you tell me why this error is coming in the checks performed? This is a different one from the previous check error, which I have linked to an open issue. I don't think its related to the changes I have made, so maybe I am just unlucky to encounter two tests failing non-deterministically🥲 for the two pushes I have made.

@cosenal
Copy link
Contributor

cosenal commented Nov 12, 2024

@Shivansh20128 https://github.com/unitaryfund/mitiq/actions/runs/11801809295/job/32876002160?pr=2566 is definitely unrelated to your changes, but it's not even a test that fails non-deterministically, or has anything to do with Mitiq. It looks like Python panicked when calling some core routine written in Rust 😱 Interesting, but definitely not in the scope of this PR 😄

@Shivansh20128
Copy link
Contributor Author

It looks like Python panicked when calling some core routine written in Rust 😱 Interesting, but definitely not in the scope of this PR 😄

Aahh. Okay. Thank you

@Shivansh20128
Copy link
Contributor Author

I think the error in the docs-build will be resolved once #2570 is merged.

@Shivansh20128 Shivansh20128 marked this pull request as ready for review November 15, 2024 17:02
@purva-thakre purva-thakre requested a review from cosenal November 15, 2024 21:55
Copy link
Contributor

@cosenal cosenal left a comment

Choose a reason for hiding this comment

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

Overall this is what we had in mind.
I agree with @purva-thakre that a snippet on how to use each circuit type would make the user guide too heavy, so let's leave only the small demo at the top.
I left some comments throughout the PR, looking forward to the next iteration.

docs/source/guide/benchmarks.md Outdated Show resolved Hide resolved
docs/source/guide/benchmarks.md Outdated Show resolved Hide resolved
docs/source/guide/benchmarks.md Outdated Show resolved Hide resolved
docs/source/guide/benchmarks.md Outdated Show resolved Hide resolved
docs/source/guide/benchmarks.md Outdated Show resolved Hide resolved
docs/source/guide/benchmarks.md Outdated Show resolved Hide resolved
docs/source/guide/benchmarks.md Outdated Show resolved Hide resolved
Shivansh20128 and others added 2 commits November 19, 2024 21:07
Co-authored-by: Alessandro Cosentino <[email protected]>
Co-authored-by: Alessandro Cosentino <[email protected]>
@Shivansh20128
Copy link
Contributor Author

Overall this is what we had in mind. I agree with @purva-thakre that a snippet on how to use each circuit type would make the user guide too heavy, so let's leave only the small demo at the top.

@cosenal Just to make sure, you want to remove the all the code snippets from the circuits and leave just the one at the top?

from mitiq.benchmarks import generate_ghz_circuit

circuit = generate_ghz_circuit(n_qubits=7)

So shall I remove all the blocks like these?

@cosenal
Copy link
Contributor

cosenal commented Nov 22, 2024

Overall this is what we had in mind. I agree with @purva-thakre that a snippet on how to use each circuit type would make the user guide too heavy, so let's leave only the small demo at the top.

@cosenal Just to make sure, you want to remove the all the code snippets from the circuits and leave just the one at the top?

from mitiq.benchmarks import generate_ghz_circuit

circuit = generate_ghz_circuit(n_qubits=7)

So shall I remove all the blocks like these?

No, those small snippets on how to generate the circuits are good. What I meant is that we shouldn't have snippets running error mitigation for each class of circuits.

@purva-thakre
Copy link
Collaborator

purva-thakre commented Nov 22, 2024

What I had in mind for how each section describing a benchmarking circuit might look like is shown below. Right now, the details pertaining to each circuit are lacking.

image

We can discuss this during the community call.

w_state_outline.txt

Edit: Note that we don't use $G(p)$ exactly as defined in the paper. There should be a line or two discussing these changes as well.

for i, j in zip(range(0, n_qubits), range(1, n_qubits)):
N = n_qubits - i
angle = 2 * np.arccos(np.sqrt(1 / N))
circuit.append(
cirq.Ry(rads=angle).controlled().on(qubits[i], qubits[j])
)
circuit.append(cirq.CNOT(qubits[j], qubits[i]))

Copy link
Collaborator

@purva-thakre purva-thakre left a comment

Choose a reason for hiding this comment

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

I suggested some changes. @Shivansh20128 let me know if you have the time to work on these.

If not, I can take over.

docs/source/guide/benchmarking-circuits.md Outdated Show resolved Hide resolved

## Mirror Circuits

The {func}`.generate_mirror_circuit` involves running a quantum circuit forward and then “mirroring” it (applying the reverse operations). Ideally, this results in returning the system to the initial state, so they’re great for testing if the noise mitigation is effective in preserving information through complex sequences.
Copy link
Collaborator

Choose a reason for hiding this comment

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

It might be better to show an example code block.

Copy link
Collaborator

@purva-thakre purva-thakre Nov 26, 2024

Choose a reason for hiding this comment

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

Most of the benchmarking circuit docstrings have a link to some reference that was used to define the function. Could you please mention these in the circuit descriptions?

https://mitiq.readthedocs.io/en/stable/apidoc.html#module-mitiq.benchmarks.mirror_circuits

The W-state outline txt file contains an example of how to use {cite} for the references listed in docs/source/refs.bib

@Shivansh20128
Copy link
Contributor Author

I suggested some changes. @Shivansh20128 let me know if you have the time to work on these.

Okay. I will make the changes.

@Shivansh20128
Copy link
Contributor Author

Hi @purva-thakre , I have added an example code block for the generate_mirror_circuit.

I have also added references for the benchmarking circuits wherever I could find them in the API-doc.

Copy link
Contributor

@cosenal cosenal left a comment

Choose a reason for hiding this comment

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

Left a small comment. The rest looks good to me.


## Mirror Quantum Volume Circuits

The {func}`.generate_mirror_qv_circuit`, as defined in {cite}`Amico_2023_arxiv`, is designed to test [Quantum Volume](https://en.wikipedia.org/wiki/Quantum_volume), a metric combining circuit depth, number of qubits, and fidelity. These circuits check whether error mitigation techniques help achieve higher effective quantum volumes on noisy devices.
Copy link
Contributor

Choose a reason for hiding this comment

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

This description sounds the same as the QV one; instead, it should be noticed that here mirror quantum volume circuits are generated with the same definition of mirroring as above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@purva-thakre purva-thakre merged commit ccbb172 into unitaryfund:main Dec 12, 2024
15 of 16 checks passed
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 benchmarks to user guide
4 participants