Flipping amplitude - #1504
Conversation
new file: src/qibocal/protocols/flipping_amplitude.py
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1504 +/- ##
==========================================
- Coverage 94.38% 93.75% -0.64%
==========================================
Files 136 137 +1
Lines 10673 10803 +130
==========================================
+ Hits 10074 10128 +54
- Misses 599 675 +76
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Maybe put some separation between the colorbar and the "Best amp" 
(Don't mind the data, there's an issue on my end that I'm debugging) |
|
Thanks for this, the fitting seems to work well 
I had to hack an implementation for sweepers because there were too many sequences to run for my hardware |
lballerio
left a comment
lballerio
left a comment
There was a problem hiding this comment.
my PR for this new interesting protocol
| # sequence.append((qd_channel, qd_detuned)) | ||
|
|
||
| if rx90: | ||
| sequence.append((qd_channel, qd_detuned)) | ||
| sequence.append((qd_channel, qd_detuned)) | ||
|
|
||
| sequence |= natives.R(theta=np.pi / 2, phi=0.0 if flips % 2 == 0 else np.pi) |
There was a problem hiding this comment.
I think we should go back to the previous configuration
| nflips_max: int = 21 | ||
| """Maximum number of flips ([RX(pi) - RX(pi)] sequences).""" | ||
| nflips_step: int = 1 | ||
| """Step size for the number of consecutive flips.""" | ||
| delta_amplitude_min: float = -0.05 | ||
| """Minimum amplitude delta relative to the native pulse amplitude.""" | ||
| delta_amplitude_max: float = 0.05 | ||
| """Maximum amplitude delta relative to the native pulse amplitude.""" | ||
| delta_amplitude_step: float = 0.001 | ||
| """Amplitude delta step.""" |
There was a problem hiding this comment.
why this coice for the default values?
There was a problem hiding this comment.
mhhh. 🥇 This is just 10% of the total range in amplitude (assuming a rabi pulse is ~ 0.3, it gives some good variability) and nflips is just a number.
| if not isinstance(self.nflips_max, int): | ||
| raise TypeError( | ||
| f"nflips_max must be int, got {type(self.nflips_max).__name__}" | ||
| ) | ||
| if not isinstance(self.nflips_step, int): | ||
| raise TypeError( | ||
| f"nflips_step must be int, got {type(self.nflips_step).__name__}" | ||
| ) | ||
| if not isinstance(self.rx90, bool): | ||
| raise TypeError(f"rx90 must be boolean, got {type(self.rx90).__name__}") |
There was a problem hiding this comment.
I think this checks can be takes as granted or this params can be recasted
There was a problem hiding this comment.
true, this I just copied from flipping but then I can make these pareameters be a subclass of FlippingParameters so that it inherits these checks.
| resonator_type: str | ||
| """Resonator type.""" |
There was a problem hiding this comment.
| resonator_type: str | |
| """Resonator type.""" |
in this protocol is not used at all, can be deleted everywhere
There was a problem hiding this comment.
true this was just me copying old code.
| """ | ||
|
|
||
| data = FlippingAmplitudeData( | ||
| resonator_type=platform.resonator_type, |
There was a problem hiding this comment.
| resonator_type=platform.resonator_type, |
as said before
| best_idx = int(np.argmin(variances)) | ||
| best_amp = float(amplitudes[best_idx]) | ||
| native_amp = data.pulse_amplitudes[qubit] | ||
|
|
||
| best_amplitudes[qubit] = [best_amp, 0.0] | ||
| delta_amplitudes[qubit] = [best_amp - native_amp, 0.0] |
There was a problem hiding this comment.
computing both best_amplitudes and delta_amplitudes is redundant, I would only use one variable, so we also delete a useless dictionary
even for the amplitude uncertainty I don't know how we can estimate it.
| amplitude=best_amplitudes, | ||
| delta_amplitude=delta_amplitudes, |
| def ev(prob: np.ndarray) -> np.ndarray: | ||
| """Helper function to calculate the expectation value.""" | ||
| return 2 * prob - 1 |
There was a problem hiding this comment.
why computing the expectation value of Z?
also I am pretty sure it should be: 1 - 2*prob, since prob is the probability of the qubit being in state 1 and for 1 <Z>=-1.
| # Build 2D probability matrix: rows = amplitude, cols = flips | ||
| z = np.full((len(amplitudes), len(flips_vals)), np.nan) | ||
| amp_index = {amp: i for i, amp in enumerate(amplitudes)} | ||
| flip_index = {fl: j for j, fl in enumerate(flips_vals)} | ||
|
|
||
| for row in qubit_data: | ||
| i = amp_index[row["amplitude"]] | ||
| j = flip_index[row["flips"]] | ||
| z[i, j] = ev(row["prob"]) |
There was a problem hiding this comment.
I think a simple reshape should be enough:
| # Build 2D probability matrix: rows = amplitude, cols = flips | |
| z = np.full((len(amplitudes), len(flips_vals)), np.nan) | |
| amp_index = {amp: i for i, amp in enumerate(amplitudes)} | |
| flip_index = {fl: j for j, fl in enumerate(flips_vals)} | |
| for row in qubit_data: | |
| i = amp_index[row["amplitude"]] | |
| j = flip_index[row["flips"]] | |
| z[i, j] = ev(row["prob"]) | |
| z = qubit_data["prob"].reshape((len(amplitudes), len(flip_vals))) |
| target, | ||
| ["Best amplitude [a.u.]", "Delta amplitude [a.u.]"], | ||
| [fit.amplitude[target], fit.delta_amplitude[target]], | ||
| display_error=True, |
There was a problem hiding this comment.
here you are displaying a 0.0 error, maybe if we don't know how to estimate the uncertainty at the moment we can neglect displaying error.
| display_error=True, | |
| display_error=False, |
|
PS maybe we should also think about new tests. |
@sorewachigauyo you mean in a separate PR? |
Yes please do the PR pointing to this one and we can combine it |
Add sweeper support for `flipping_amplitude`
|
Hi @jevillegasd, I've just merged the sweeper implementation of the |
3 participants
This is a 2D sweep for the flipping routine in which a amplitude detuning is used in one axis.