Convert all QM sweepers to absolute

[stavros11]

Following the discussion in #540, this PR makes all QM sweepers absolute. Since SweeperType is already dropped in 0.2, this will also close the issue.

I tested on hardware with the Rabi routine using an absolute amplitude sweeper: http://login.qrccluster.com:9000/XftyLYqDSsG8xBCkcz6ibg==
In this case, the same amplitude range is used on all qubits. It is possible to use a different range per qubit using parallel sweepers (#1008), but I have not tested this.

@alecandido @andrea-pasquale note that although all amplitude sweepers are now absolute (no more factors and multiplications), flux pulses are always played on top of the DC offset (sweetspot). This is true even when not sweeping. Therefore, amplitude sweeps on flux pulses will also be played on top of the DC offset. I could change this behavior to zero the offset when we are sweeping flux pulses, however I don't think we want that, since in most cases we are operating all qubits at their sweetspot. Moreover, we already provide an interface to temporarily change the DC offset for one execution, by passing an alternate DcConfig when calling platform.execute.

EDIT: In contrast, Parameter.offset sweeps are really absolute, not around the sweetspot like in 0.1, because there we are sweeping the offset value directly (not a pulse amplitude on top).

[codecov]

Codecov Report

Attention: Patch coverage is 5.06329% with 75 lines in your changes missing coverage. Please review.

Project coverage is 51.71%. Comparing base (cd6dcd6) to head (cef71e4).
Report is 11 commits behind head on 0.2.

Files with missing lines	Patch %	Lines
src/qibolab/instruments/qm/program/sweepers.py	0.00%	35 Missing ⚠️
src/qibolab/instruments/qm/controller.py	0.00%	27 Missing ⚠️
src/qibolab/instruments/qm/program/instructions.py	0.00%	8 Missing ⚠️
src/qibolab/instruments/qm/program/arguments.py	0.00%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              0.2    #1018      +/-   ##
==========================================
- Coverage   51.86%   51.71%   -0.16%     
==========================================
  Files          58       58              
  Lines        2765     2777      +12     
==========================================
+ Hits         1434     1436       +2     
- Misses       1331     1341      +10     

Flag	Coverage Δ
unittests	51.71% <5.06%> (-0.16%)	⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[andrea-pasquale]

@alecandido @andrea-pasquale note that although all amplitude sweepers are now absolute (no more factors and multiplications), flux pulses are always played on top of the DC offset (sweetspot). This is true even when not sweeping. Therefore, amplitude sweeps on flux pulses will also be played on top of the DC offset. I could change this behavior to zero the offset when we are sweeping flux pulses, however I don't think we want that, since in most cases we are operating all qubits at their sweetspot. Moreover, we already provide an interface to temporarily change the DC offset for one execution, by passing an alternate DcConfig when calling platform.execute.

Thanks for the PR @stavros11.
I'm also fine in keeping flux pulse amplitudes relative to the DC offset. As you said for most applications it should be ok to assume that we are putting the qubit at a specific DC offset and then you move around this point using flux pulses.

[alecandido]

I would also say that even that is "absolute". Not the absolute value on the line, but that's the absolute value of the amplitude of the flux pulse itself, that's exactly the sweeper parameter. The offset is a different thing, generated differently (not "pulsed"), though on the same line.

[alecandido]

Just to understand: while I'm fine with lifting the check from the frequency sweeper method earlier (such that _frequency is more minimal and standard, and the check happens as early as possible), I'm not sure I understood why now you need to preprocess amplitude sweepers, while before you didn't.

True that you're now normalizing the amplitude, but this seems an element-wise operation...
I'm not even sure why you need the max(abs(values)) in sweeper_amplitude, since I just expected the normalization with the maximum range (i.e. just the division by a constant).

[stavros11]

Just to understand: while I'm fine with lifting the check from the frequency sweeper method earlier (such that _frequency is more minimal and standard, and the check happens as early as possible),

Another reason I moved this was to eliminate most arguments in the sweeper functions (_frequency, _amplitude, etc.). For example, this was the only case where values was used in these functions. I still didn't have to lift it all the way up to QmController for that, but it looked better this way.

True that you're now normalizing the amplitude, but this seems an element-wise operation... I'm not even sure why you need the max(abs(values)) in sweeper_amplitude, since I just expected the normalization with the maximum range (i.e. just the division by a constant).

I probably didn't document it very well and maybe there are different ways to make it work. In summary, the issue is that the amplitude of the original pulse may not be sufficient to reach all the values in the sweeper, because qua.amp that is needed to modify amplitude in real-time is limited by < 2 (see https://docs.quantum-machines.co/1.2.0/docs/API_references/qua/dsl_main/#qm.qua._dsl.amp).
For example, if the original pulse in the sequence has amplitude 0.05, then QUA can sweep it up to amplitude ~2*0.05=0.1 in real-time. If the user asks for values up to, say, 0.4 in the sweeper, which are still valid since are < 1, QUA will fail (or worse return random stuff). My solution was to register a copy of the original pulse with amplitude max(abs(sweeper.values)) / 1.99 in the config and sweep over that, because this way we are sure that QUA is able to reach all required sweeper values in real-time (up to 2^(-16) precision which is typically fine).

[alecandido]

Another reason I moved this was to eliminate most arguments in the sweeper functions (_frequency, _amplitude, etc.). For example, this was the only case where values was used in these functions. I still didn't have to lift it all the way up to QmController for that, but it looked better this way.

Yes, that's what I meant by minimal and standard.

I probably didn't document it very well and maybe there are different ways to make it work. In summary, the issue is that the amplitude of the original pulse may not be sufficient to reach all the values in the sweeper, because qua.amp that is needed to modify amplitude in real-time is limited by < 2 (see docs.quantum-machines.co/1.2.0/docs/API_references/qua/dsl_main#qm.qua._dsl.amp).
For example, if the original pulse in the sequence has amplitude 0.05, then QUA can sweep it up to amplitude ~2*0.05=0.1 in real-time. If the user asks for values up to, say, 0.4 in the sweeper, which are still valid since are < 1, QUA will fail (or worse return random stuff). My solution was to register a copy of the original pulse with amplitude max(abs(sweeper.values)) / 1.99 in the config and sweep over that, because this way we are sure that QUA is able to reach all required sweeper values in real-time

Perfect, thanks for the explanation.

(up to 2^(-16) precision which is typically fine).

It's almost 10^-5, I'd say we really don't have that sensitivity...

[stavros11]

I see that #1001 is also rebased on top of this and it works on hardware so I’ll merge. Further testing would help to make sure that different sweep cases (including nested and parallel combinations) work as expected but we’re already aware that we need to improve testing in 0.2.