Fix flipping amplitude correction sign #716
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #716 +/- ##
=======================================
Coverage 97.19% 97.19%
=======================================
Files 110 110
Lines 7618 7618
=======================================
Hits 7404 7404
Misses 214 214
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
We have just made the release 😞 |
|
I know, they were located while running in hardware. |
| @@ -178,7 +178,7 @@ def _fit(data: FlippingSignalData) -> FlippingSignalResults: | |||
| popt[3] - x_min * 2 * np.pi * f / (x_max - x_min) * popt[2], | |||
| popt[4] * 2 * np.pi * f / (x_max - x_min), | |||
| ] | |||
| if popt[3] > np.pi / 2 and popt[3] < 3 * np.pi / 2: | |||
| if translated_popt[3] > np.pi / 2 and translated_popt[3] < 3 * np.pi / 2: | |||
There was a problem hiding this comment.
I agree with all other changes, I need to check this one, but it seems correct.
There was a problem hiding this comment.
I think this was related to ramsey giving the correction in the wrong direction qiboteam/qibolab_platforms_qrc#119 (comment)
There was a problem hiding this comment.
Uhm, but this is flipping it is not ramsey...
There was a problem hiding this comment.
Yes sorry,
I think this was related to ramsey flipping giving the correction in the wrong direction.
There was a problem hiding this comment.
Looks good to me. Let's confirm @andrea-pasquale's comments and we should be able to merge.
|
I manage correct the formula for the flipping. I am not sure if I hardcoded something related to the fact that this is a cavity and not a 2D resonator. Here are some reports: |
I can tell that the fits are fine, but have you checked that the amplitude correction was in the right direction, if so, let's merge it. |
|
I believe I checked but in some of the reports the amplitude correction seems to be wrong. It might be because I was just running qq fit and not updating the report. Let me double check. |
Yes, one of the report was outdated, everything else works. I'll go ahead and merge. |
andrea-pasquale
changed the title
I will, but they should not overlap as we only changed the fitting here and acquisition on the other one, feel free to merge #720 as well. |
|
Yes, you are right. I just wanted to make sure that we can reproduce the same results with and without unrolling. |
@Edoardo-Pedicillo possible issues on the flipping, can you have a look and confirm them ?
Checklist:
mastermainmain