Hotfix for rabi period estimate

[andrea-pasquale]

Closes #656.
I think there was a problem with some simplification in the correction formula for the rabi period estimate.

Checklist:

• Reviewers confirm new code works as expected.
• Tests are passing.
• Coverage does not decrease.
• Documentation is updated.
• Compatibility with Qibo modules (Please edit this section if the current pull request is not compatible with the following branches).
  • Qibo: master
  • Qibolab: main
  • Qibolab_platforms_qrc: main

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5f10133) 96.02% compared to head (2e48250) 96.02%.
Report is 4 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #677   +/-   ##
=======================================
  Coverage   96.02%   96.02%           
=======================================
  Files         106      106           
  Lines        7354     7355    +1     
=======================================
+ Hits         7062     7063    +1     
  Misses        292      292           

Flag	Coverage Δ
unittests	96.02% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...c/qibocal/protocols/characterization/rabi/utils.py	100.00% <100.00%> (ø)

[alecandido]

My bad, the two functions are actually equal only in a certain range, and I see why you want the current one

I should have tested better...

Another option, that is exactly what you're doing, would be:

1.5 - ((x + 0.5) % 1)

But it is really a matter of taste...

[andrea-pasquale]

Thanks for checking @alecandido.
Indeed I saw the same with behavior with np.modf. For the time being let's keep np.round.

[alecandido]

Btw, is there a reason why you fixed the codomain between 0.5 and 1.5?

[andrea-pasquale]

Btw, is there a reason why you fixed the codomain between 0.5 and 1.5?

Which codomain?

[alecandido]

Which codomain?

The y-axis in the plot (the codomain of the function you're modifying in this PR).

Essentially, the amplitude is fixed to be 1 by np.round() (and all its friends), and you're deciding to center the range at 0, and to make f(0) = 1.

[andrea-pasquale]

Which codomain?

The y-axis in the plot (the codomain of the function you're modifying in this PR).

Essentially, the amplitude is fixed to be 1 by np.round() (and all its friends), and you're deciding to center the range at 0, and to make f(0) = 1.

I see. Maybe it is better to go a step behind. The (half-)period corrected taking into account the phase should be the following:

$$ \frac{T^*}{2} = \underbracket{\Big( k - \frac{\phi}{\pi}\Big) }_{\xi} \frac{T}{2} $$

and since I now that this correction will be small I want to find $k \in \mathbb{Z}$ which minimizes this correction. Which means that I need to find $k$ s.t. $k - \frac{\phi}{\pi} \approx 1$. Therefore $k = \text{int}( 1 + \phi / \pi)$ and the total correction will be $\xi = \text{int}( 1 + \phi / \pi) - \phi/\pi$. The problem as far as I understood is how to treat this conversion to int.

Given that we want $k$ to be as close as possible to the float this is why I think that it is better to use np.round instead of np.modf. Here are more plots.

[alecandido]

Yes, I keep forgetting it is a multiplicative correction, that's why you want the value to be around 1.