Add sweeper types

[rodolfocarobene]

Adding a type parameter to sweepers, so that it's possible to choose if the sweeper should be relative-multiplied, relative-summed or absolute.
Still nothing was tested with qibocal, I'll open a PR.

But before, If someone has some comment on the implementation please tell me.
I coded the type enum so that it add "natively" the operation to execute

import operator
from functools import partial
class SweeperType(Enum):
    """Type of the Sweeper"""

    ABSOLUTE = partial(lambda x, y=None: x)
    FACTOR = operator.mul
    OFFSET = operator.add

This enables to use the same interface for the three cases. For example, from the Rfsoc driver:

value = qubit.flux.bias
start = sweeper.type.value(sweeper.values[0], value)
stop = sweeper.type.value(sweeper.values[-1], value)

Checklist:

• Reviewers confirm new code works as expected.
• Tests are passing.
• Coverage does not decrease.
• Documentation is updated.

[codecov]

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.06 🎉

Comparison is base (4767ca5) 57.59% compared to head (54c0476) 57.66%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #459      +/-   ##
==========================================
+ Coverage   57.59%   57.66%   +0.06%     
==========================================
  Files          37       37              
  Lines        6198     6205       +7     
==========================================
+ Hits         3570     3578       +8     
+ Misses       2628     2627       -1     

Flag	Coverage Δ
unittests	57.66% <100.00%> (+0.06%)	⬆️

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

Impacted Files	Coverage Δ
src/qibolab/instruments/rfsoc.py	56.97% <100.00%> (+0.05%)	⬆️
src/qibolab/sweeper.py	100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[Jacfomg]

Seems cool to me, the biggest thing would be also deciding how each sweeper will behave on the existing routines.

[rodolfocarobene]

Seems cool to me, the biggest thing would be also deciding how each sweeper will behave on the existing routines.

Thank you @Jacfomg, I think that what qibocal should do is another issue... I opened a PR in qibocal #369 if you want to propose things.

In any case, this would require a driver update, I was able to update the rfsoc one, but the others are missing.
Also, this PR is pointing to the new rfsoc driver, not directly to main... So there is time to fix things

[andrea-pasquale]

Thanks @rodolfocarobene.
This is a nice idea, especially if we want to implement more than one way of sweeping a parameter without having to duplicate code.
If I understand correctly this feature will not break the other drivers but if they implement the same approach that you used her it will be possible to change in qibocal the way in which you sweep by changing the sweeper type, correct?

[rodolfocarobene]

If I understand correctly this feature will not break the other drivers but if they implement the same approach that you used her it will be possible to change in qibocal the way in which you sweep by changing the sweeper type, correct?

Yes, that's correct. If we merge this as it is the drivers will just continue to use the sweepers as before, not considering the "type" of the sweeper.
Note also that the default value of type is ABSOLUTE so if only my driver supports it and qibocal does not get updated it could be a problem...

I think we can merge this just after the zcu111 driver

[andrea-pasquale]

@rodolfocarobene given that you have already 4 approving reviews feel free to merge this whenever you want :)
After you do we should be able to merge also qiboteam/qibocal#369.

[aorgazf]

Thanks @rodolfocarobene for helping with this.
As discussed, I was wondering if there would be an alternative that reads clearer.
The driver needs the absolute_values to be swept. To calculate them, one needs the relative_values, the type of operation to be done with them (these two pieces of information are in the Sweeper), and the base_value which is available within the driver scope.

I was wondering if there is a clearer way.
How about this:

class Sweeper:
    values: npt.NDArray
    def calculate_absolute_values(self, base_value):
        return self.type.value(self.values, base_value)
# driver code
if sweeper.type == SweeperType.ABSOLUTE:
    values = sweeper.values
else:
    values = sweeper.calculate_absolute_values(base_value) 

If we wanted to remove the if statement within the driver and always use the same call (irrespective of the type) we may as well rename it as values:

class Sweeper:
    _values: npt.NDArray
    def values(self, base_value):
        return self.type.value(self._values, base_value)

# driver code
values = sweeper.values(base_value)

@alecandido what do you think?

[alecandido]

Thanks, @aorgazf, for the proposal.

I definitely like the idea of base values, it is fine to define the operand, but at the moment they were mainly (symmetric) binary functions, apart from the absolute.
And it's true that instead there is no symmetry, since one is the array to sweep, and the other one can only be a single number. But this was only enforced at individual drivers level, with no explicit common sorting (only enforced now implicitly by SweepType.ABSOLUTE value).

In general, I believe this bit could (and should) also be encoded in this PR (since it's strictly related), but we definitely need a more thorough discussion about sweepers standardization, even in light of some of the ideas of @stavros11 and @maxhant (respectively: per-pulse sweep values and multi-parameter sweepers).

Among the two alternatives proposed, I like more the second one (of course ^^). Not much because of the fewer lines of code (that, when explicit enough, I always like), but because there is less code (i.e. tasks) delegated to the driver specific implementation.

[rodolfocarobene]

I added the values(self, base_value) function. Tests are failing for the dependency problem in zcu111-server

[alecandido]

Tests are failing for a reason already reported in #373.

For sure they should pass before merging, but for me the update is fine.

[alecandido]

Btw: do not merge this before #373. They are both ready, so I'd merge both in main.

If instead you merge first this in the other, and then you might want to change something, the content of #373 will grow, and essentially we'll be rereviewing this PR.

[rodolfocarobene]

I'll wait for the tests to pass for merging