-
Notifications
You must be signed in to change notification settings - Fork 8
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
docs(notebooks): add estimator utility scaling demo #28
Conversation
Pull Request Test Coverage Report for Build 8621923842Details
💛 - Coveralls |
This is looking great, @ashsaki. I really like the new plots that you are introducing although some of them we maybe want to study further (e.g. why DD worsens the results?) Just as a suggestion, it would be great if we could still reproduce some of the same plots from the original scaling notebook here for comparison. |
Also, @ashsaki , can you move the notebook outside of the We need to think of short, distinct names for the two notebooks 🙂 |
We have decided on the following names:
|
Results are looking correct now.
We cannot reproduce the original plots at different noise factors, because the noisy expectation values are only reported through experimental settings (which are discouraged) |
I did a new run to separate out the The PR is now up for review @pedrorrivero. |
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.
Thanks @ashsaki , excellent work! 🥳
A few comments to address:
- You can add our names as authors if you want, in the same style as we have for
scaling-sampler
(maybe avoid emails) - Notice that in the
scaling-sampler
notebook we declare thecompute-uncompute
function directly in the notebook instead of importing it. Please do so here as well, and make sure that there are no other code discrepancies. - Delete
instance = ...
. - Delete all commented-out code that is not meant to be uncommented (e.g. different backends)
- We need to clarify that Runtime does everything for us, and that we are only using other tools for illustration purposes (e.g. DD, PT, ZNE).
- Note at the very beginning of the notebook.
- Wherever we use some other tool.
- Point out in Step 3 that the circuits to be submitted are those without any modifications (e.g. no DD, PT, ZNE).
- When explaining TREX, I think it would be better to explain it in terms twirled measurements and calibrations, as it connects with the Runtime options as well as with the previously introduced concept of twirling, and it also teaches about the needed quantum overhead. We can work together on this if you want.
- ISA stands for Instruction Set Architecture (i.e. basis gates and topology). It affects the observables only secondhand through the effects that take place on the circuits themselves (e.g. circuit layout, routing). Therefore, I am not too convinced about the notion of ISA observables.
- Introduce ISA in the transpilation section.
- Indicate the version of Runtime for which you are listing out options.
- In your tip re/ Runtime's
Session
it might be more useful to mention execution modes in general and link to the relevant docs. - For this use case
Batch
mode is better than session. - We must encourage people to close the Runtime session/batch! The easiest way to do this is by opening it in a context manager:
with Session(...) as session:
. Alternatively you can callsession.close()
.- Close right after submission.
- Remove "optional" word.
- Say that the session will close automatically after the iTTL, but that closing manually is encouraged for efficiency and to avoid unnecessary waiting time.
- Explain that for a better programming experience it is better to use a context manager and point to documentation → maybe we should use it as well, this would place all execution commands in one cell and avoid the risk of users running the notebook exceeding the iTTL.
- Add hyperlink to docs explaining iTTL, instead of just pasting the URL.
- Remove unnecessary comment for
estimator.options.resilience_level = 0
- Question: when you do
estimator.options.resilience_level = 1
do you override previously defined (advanced) settings? - Update: "Each
PrimitiveResult
object has a list-like structure holdingPubResult
s. Common metadata is also available inside ofPrimitiveResult
." - Write the full word "No-mitigation" in the plots
- I think it would be better to show weight-1, weight-2 and average in the same plot. you don't need to specify this in the title, it is already clear in the legend.
- Remove the two M3 references and add TREX.
Also, what is this for? Does it make sense to keep it for users to know about it?
%config InlineBackend.figure_format='retina'
1862cef
to
9e192c7
Compare
Omitted as most other notebooks do not have author names.
Done. Let me know if you find other discrepancies.
Deleted.
Deleted.
Done. (too many pointing outs now?)
Need help. Will work together.
Agree. Changed to
Done.
Did not understand completely. We are listing software dependencies at the end of the notebook. Do we need to call
Done. Removed the tip banner and replaced it with texts that mentions execution modes
Switched to
Used context manager which automatically handles the closing.
Not relevant anymore as switched to
Not relevant anymore due to switch to
Removed.
Not affirmative. In this notebook, advanced settings are coming later. If needed, we can find the answer separately.
Updated.
Done.
Done. Merged two plots into one.
Found one reference to M3 and removed that. Could not find the second reference.
This is for high resolution plots. Removed as it is a nicety not a necessity. |
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.
Looking better! Final remarks:
- Let's go together about the notes on external tools in a call and get to the right amount and style
- Use consistent color scheme for color boxes:
- Note → blue
- Warning → yellow
- Tip → green
- Danger → red
- Use consistent naming and capitalization: Qiskit Runtime
- In the
EstimatorV2
section, where you present the options, please say that those are the options as of Runtime versionX.Y.Z
- Move TREX reference up, between current references [5] and [6]
- Regarding the text for TREX how about:
Twirled readout error extinction (TREX) is an efficient method for mitigating measurement errors on expectation value problems for Pauli observables. It is based on the notion of twirled measurements, which are accomplished by randomly substituting measurement gates by a sequence of (1) a Pauli-X gate, (2) a measurement, and (3) classical bit flip. Just like in standard gate twirling, this sequence amounts to the same as just a single measurement in the absence of noise; otherwise, it will have the effect of diagonalizing the readout-error transfer matrix, making it trivial to invert. To find such inverse, identity circuits with random measurement twirls need to be executed for calibration, which induces overhead on the quantum hardware.
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.
Great job @ashsaki! LGTM 🥳
Summary
This PR creates a variant of the scaling notebook using Qiskit Runtime's
EstimatorV2
and its built-in error mitigation capabilities.Details and comments
The notebook shows how configure
EstimatorV2
s built-in error mitigation options to run any circuit at scale. It covers following runs and compares the results.