-
-
Notifications
You must be signed in to change notification settings - Fork 163
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
2274 Benchmark User Guide #2566
Conversation
There was a problem hiding this 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.
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. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
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. |
@Shivansh20128 this is exactly the kind of thing we're looking for! |
@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. |
@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 😄 |
Aahh. Okay. Thank you |
I think the error in the docs-build will be resolved once #2570 is merged. |
There was a problem hiding this 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.
Co-authored-by: Alessandro Cosentino <[email protected]>
Co-authored-by: Alessandro Cosentino <[email protected]>
@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?
So shall I remove all the blocks like these? |
Co-authored-by: Purva Thakre <[email protected]>
…sh20128/mitiq into 2274-benchmark-user-guide
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. |
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. We can discuss this during the community call. Edit: Note that we don't use mitiq/mitiq/benchmarks/w_state_circuits.py Lines 38 to 44 in edeede6
|
There was a problem hiding this 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.
|
||
## 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. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
Okay. I will make the changes. |
Co-authored-by: Purva Thakre <[email protected]>
Hi @purva-thakre , I have added an example code block for the I have also added references for the benchmarking circuits wherever I could find them in the API-doc. |
There was a problem hiding this 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. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
Description
This PR adds a user guide section for benchmarks.
Closes issue #2274
License
Before opening the PR, please ensure you have completed the following where appropriate.