-
Notifications
You must be signed in to change notification settings - Fork 127
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
Drag analysis #732
Drag analysis #732
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.
Can you add some more test cases? In particular, I think it would be good to add:
- A case where the amplitude varies only between 0 and 0.5 to account for the SX calibration
- A case where the ideal beta is more than half a period away from 0 for the 1 rep case.
Regarding the second case, here is an example that fits well with the current code but not well with this PR:
from qiskit_experiments.library import RoughDrag
from qiskit_experiments.test.mock_iq_backend import DragBackend
from qiskit import pulse
from qiskit.circuit import Parameter
from qiskit.pulse import DriveChannel, Drag
import numpy as np
beta = Parameter("β")
with pulse.build(name="xp") as xp:
pulse.play(Drag(duration=160, amp=0.208519, sigma=40, beta=beta), DriveChannel(0))
backend = DragBackend(gate_name="Drag(xp)", ideal_beta=9)
drag = RoughDrag(1, xp, betas=np.linspace(-10, 10, 101))
expdata = drag.run(backend)
I believe this is because this PR allows for both signs of the amplitude which leads to the fit trying to match the peak in the 1 rep case instead of the dip. -- This is actually because the range of beta values is restricted to less than the swept range in the PR.
One other minor comment -- the DragBackend
is pretty misleading. When checking for amp sign fitting issues, it took me a while to figure out what the scaling factor is on error. It gets scaled bysum(circuit.count_ops().values())
which is 2 x drag + 2 x u1 + barrier + measure = 6. It feels like the error should get scaled by the number of drag gates instead, if not drag gates / 2.
qiskit_experiments/library/characterization/analysis/drag_analysis.py
Outdated
Show resolved
Hide resolved
qiskit_experiments/library/characterization/analysis/drag_analysis.py
Outdated
Show resolved
Hide resolved
qiskit_experiments/library/characterization/analysis/drag_analysis.py
Outdated
Show resolved
Hide resolved
…xperiments into drag_analysis_fit_guess
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 have some minor comments, but looks good!
qiskit_experiments/library/characterization/analysis/drag_analysis.py
Outdated
Show resolved
Hide resolved
qiskit_experiments/library/characterization/analysis/drag_analysis.py
Outdated
Show resolved
Hide resolved
qiskit_experiments/library/characterization/analysis/drag_analysis.py
Outdated
Show resolved
Hide resolved
qiskit_experiments/library/characterization/analysis/drag_analysis.py
Outdated
Show resolved
Hide resolved
qiskit_experiments/library/characterization/analysis/drag_analysis.py
Outdated
Show resolved
Hide resolved
qiskit_experiments/library/characterization/analysis/drag_analysis.py
Outdated
Show resolved
Hide resolved
…ysis.py Co-authored-by: Will Shanks <[email protected]>
…ysis.py Co-authored-by: Will Shanks <[email protected]>
…ysis.py Co-authored-by: Will Shanks <[email protected]>
Co-authored-by: Will Shanks <[email protected]>
* Made a nasty test less nasty.
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.
More minor comments, but I think this is almost done.
qiskit_experiments/library/characterization/analysis/drag_analysis.py
Outdated
Show resolved
Hide resolved
qiskit_experiments/library/characterization/analysis/drag_analysis.py
Outdated
Show resolved
Hide resolved
qiskit_experiments/library/characterization/analysis/drag_analysis.py
Outdated
Show resolved
Hide resolved
qiskit_experiments/library/characterization/analysis/drag_analysis.py
Outdated
Show resolved
Hide resolved
…ysis.py Co-authored-by: Will Shanks <[email protected]>
…ysis.py Co-authored-by: Will Shanks <[email protected]>
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 had one small wording suggestion in line but this looks good to me.
qiskit_experiments/library/characterization/analysis/drag_analysis.py
Outdated
Show resolved
Hide resolved
…ysis.py Co-authored-by: Will Shanks <[email protected]>
* The DragAnalysis class has been made more robust. The initial guess for the fit parameters have been improved and more tests were added. Furthermore, a fit post processing hook has been added to curve analysis to allow Drag to report drag parameters in the `+-1 / freq` range. Co-authored-by: Will Shanks <[email protected]>
Summary
This PR makes the Drag analysis less prone to fit failures by improving the initial guesses.
Details and comments
The initial guesses for the fit amplitudes are adjusted as well as for the base of the fit function. This addresses #685 . This PR also changes the internals of the drag analysis: the three frequencies are replaced with a single frequency which is multiplied by the number of drag gate repetitions. This is done by introducing
reps
as a fixed parameter. Furthermore, the frequency is estimated based on the fastest oscillation. Furthermore this PR introduces several tests of challenging data: