RX90 calibration implementation

[ElStabilini]

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

Attention: Patch coverage is 91.96429% with 9 lines in your changes missing coverage. Please review.

Project coverage is 96.84%. Comparing base (c280398) to head (7b907ae).
Report is 3 commits behind head on 0.2.

Files with missing lines	Patch %	Lines
src/qibocal/protocols/flipping.py	67.85%	9 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              0.2    #1044      +/-   ##
==========================================
- Coverage   97.02%   96.84%   -0.18%     
==========================================
  Files          98       98              
  Lines        7893     7957      +64     
==========================================
+ Hits         7658     7706      +48     
- Misses        235      251      +16     

Flag	Coverage Δ
unittests	96.84% <91.96%> (-0.18%)	⬇️

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

Files with missing lines	Coverage Δ
src/qibocal/protocols/rabi/amplitude.py	96.87% <100.00%> (+0.10%)	⬆️
src/qibocal/protocols/rabi/amplitude_frequency.py	97.95% <100.00%> (+0.02%)	⬆️
...bocal/protocols/rabi/amplitude_frequency_signal.py	98.46% <100.00%> (+0.08%)	⬆️
src/qibocal/protocols/rabi/amplitude_signal.py	97.59% <100.00%> (+0.18%)	⬆️
src/qibocal/protocols/rabi/ef.py	100.00% <100.00%> (ø)
src/qibocal/protocols/rabi/length.py	96.20% <100.00%> (+0.09%)	⬆️
src/qibocal/protocols/rabi/length_frequency.py	97.05% <100.00%> (+0.02%)	⬆️
.../qibocal/protocols/rabi/length_frequency_signal.py	97.79% <100.00%> (+0.11%)	⬆️
src/qibocal/protocols/rabi/length_signal.py	96.59% <100.00%> (+0.24%)	⬆️
src/qibocal/protocols/rabi/utils.py	99.20% <100.00%> (+0.06%)	⬆️
... and 2 more

... and 3 files with indirect coverage changes

[ElStabilini]

I attach here some reports where I tested the code:

• rabi_amplitude routine: RX($pi$), RX($\pi/2$)
• rabi_amplitude_frequency_routine: RX($\pi$), RX($\pi/2$)
• rabi_length routine: RX($\pi$), RX($\pi/2$)
  All $\pi/2$ routines were executed using the main platform in and the code from ec4b7ad, while $\pi$ routines were run using qibocal branch 0.2

[andrea-pasquale]

Thanks @ElStabilini, the results seem to make sense since for both amplitude and duration there is a factor 2. I think that rather than changing the protocol completly we could consider adding a flag to specify whether we want to calibrate the RX or RX90 pulse. We should also check the corresponding update function in the protocols to be consistent.
Eventually we can do the same for both the flipping and the drag calibration. For now let's finish first rabi.

[ElStabilini]

Thanks @ElStabilini, the results seem to make sense since for both amplitude and duration there is a factor 2. I think that rather than changing the protocol completly we could consider adding a flag to specify whether we want to calibrate the RX or RX90 pulse. We should also check the corresponding update function in the protocols to be consistent. Eventually we can do the same for both the flipping and the drag calibration. For now let's finish first rabi.

@andrea-pasquale I see this only now, I should have done this by adding the pihalf_pulse (soon rx90) option, correct?
Regarding routines, should I change them even though I added a new RX90 native gate and left untouched the RX?

[andrea-pasquale]

Thanks for the updates @ElStabilini.
At this point I would rename everywhere pihalf_pulse to rx90.
Everything else looks good to me.

[andrea-pasquale]

Here we should force the user to calibrate just with RX, given that we have no RX1290 pulse.

[ElStabilini]

I should have fixed this too, in this commit 349e312.

If the user sets rx90: True it will raise an error

[andrea-pasquale]

This line needs to be fixed.

[ElStabilini]

I should have fixed it and also added the figure

[andrea-pasquale]

Regarding routines, should I change them even though I added a new RX90 native gate and left untouched the RX?

Just to avoid too many things at the same time I suggest to focus first on rabi experiments. In another PR we can propagate these changes to the other protocols.

[ElStabilini]

I copy here a commit message I left in c579cbb

The problem here is that right now we have the possibility to calibrate two different gates RX and RX90 so the _update function is defined accordingly taking rx90: bool as input.

Right now I simply added the input rx90 = False but maybe in the future can be useful to modify the flipping protocol in order to be able to run it for both RX and RX90 gates.

[andrea-pasquale]

The problem here is that right now we have the possibility to calibrate two different gates RX and RX90 so the _update function is defined accordingly taking rx90: bool as input.

Right now I simply added the input rx90 = False but maybe in the future can be useful to modify the flipping protocol in order to be able to run it for both RX and RX90 gates.

This is fine for now, eventually we could even put as default rx90=True in the update functions. In this way all other protocols that were using these functions will remain compatible.

[alecandido]

Someone noticed a certain streak of pretty close commit messages (once in a while, I was not the first one - I won't take this merit :P).

Commit messages are extremely useful for debugging, to navigate the commits in case something goes wrong. So, ideally, they should describe in the most succinct and expressive way the commits' changes.
Think about searching for some change by yourself, which you don't remember exactly when and where it happened, or if you need to reprocess commits (e.g. during a rebase).

Of course, sometimes you need to push to debug the workflow. But this is better done with a local reproduction as much as possible. And, if really needed, possibly even squashing some failed attempts afterward, to leave a better history for later :)

[alecandido]

In these cases, it is especially convenient to use the ternary if operator (exact same of the ... ? ... : ... construct in C)

Suggested change

	if data.rx90:
	pulse_name = "Pi-half pulse"
	else:
	pulse_name = "Pi pulse"
	pulse_name = "Pi-half pulse" if data.rx90 else "Pi pulse"

[alecandido]

@ElStabilini just to provide you some context about #1044 (comment), the clear advantage of the proposed syntax is neither saving characters nor lines.

The most prominent difference between the equivalent expressions is that one is imperative, the other functional.
In the first one, you or conditioning the entire assignment operation, as the process of defining a variable name and its associated value. In principle, you need to analyze both branches of the conditional to ensure that the variable (pulse_name in this case) is even defined.
Instead, when using the ternary operation you are not conditioning the assignment as an operation, which becomes just an alias for a given computation. The computation is an expression, that always has a value. But it's the value itself the one being conditioned.

The functional approach makes the computation flow clearer, since it is somehow more rigid (you're not even thinking about operations, and possibly memory, but just values manipulation), and makes the analysis simpler and more effective. In languages which are actually functional, this may empower the compiler to act more aggressively. In Python, it makes the linters more effective[*], leading to fewer errors and more helpful messages :)

[*]: since it optimizes the simplicity of the code, it makes it also more human-readable in general

[ElStabilini]

@ElStabilini just to provide you some context about #1044 (comment), the clear advantage of the proposed syntax is neither saving characters nor lines.

The most prominent difference between the equivalent expressions is that one is imperative, the other functional. In the first one, you or conditioning the entire assignment operation, as the process of defining a variable name and its associated value. In principle, you need to analyze both branches of the conditional to ensure that the variable (pulse_name in this case) is even defined. Instead, when using the ternary operation you are not conditioning the assignment as an operation, which becomes just an alias for a given computation. The computation is an expression, that always has a value. But it's the value itself the one being conditioned.

The functional approach makes the computation flow clearer, since it is somehow more rigid (you're not even thinking about operations, and possibly memory, but just values manipulation), and makes the analysis simpler and more effective. In languages which are actually functional, this may empower the compiler to act more aggressively. In Python, it makes the linters more effective[*], leading to fewer errors and more helpful messages :)

[*]: since it optimizes the simplicity of the code, it makes it also more human-readable in general

It definitely makes sense, thank you!