-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Drop SweeperType
#540
Comments
It's on my todo list #535. But, I will prioritize doing things with the chip before it goes away. |
Yes, I am also planning to do it. |
Perfect, thank you! |
@stavros11 @Jacfomg @andrea-pasquale |
I fully agree with the first part. However, how to implement the sweep (how to transfer the parameters, and possibly further details about the sweeper implementation) will of course be instrument specific. But the most we can implement in the common layer, the better it is. If there is enough consensus, change the assigned milestone. |
Fine also from my side, the motivation to open the issue was to have a common interface in |
Since everyone agrees, I moved this to the 0.2.0. And I also agree with lifting as much code as possible to the common layer. |
Actually, is it convenient for the devices to use the The alternative would be to just use |
The main reason why sweepertype different from absolute exist is because of multiple qubits calibration. This was already done before, but implicitly. From the QICK point of view, there is no difference in memory usage or anything else. Actually what I implement is always an "absolute"-like sweeper |
Ok, but then this doesn't have to be a driver-supported feature, but it could be even a Qibocal feature. |
To answer this, Following @rodolfocarobene comment, the main use of However, regarding QM in particular, parallel sweeps are compiled using the Potential solutions for 0.2:
|
I believe this was the kind of applications for which qibolab/src/qibolab/instruments/icarusqfpga.py Lines 419 to 423 in bbac7cf
which makes it useless, since it could be done on the Qibocal side (if needed).
QM 0.1 is not even supporting sweeper types, and deciding the "type" based on the parameter. E.g. qibolab/src/qibolab/instruments/qm/sweepers.py Lines 97 to 100 in 9bbbbe8
(this is the frequency, and it's a SweeperType.OFFSET , always - irrespective of the actual SweeperType set - but there is a similar for_ loop in all the other parameters)
So, since we'd really like to release 0.2 at this point, I would keep the minimal feature right now, and keep performance optimizations for later.
I would keep 2. and 3. for 0.2.2 or later. In the meanwhile, just drop it, and follow the 0.1 approach. We also need to support I'd keep the 0.1 behavior even in 0.2, but let's switch input: class Sweeper(Model):
...
linspace: Optional[tuple[float, float, float]] = None
values: Optional[npt.NDArray] = None
...
@model_validator(mode='after')
def check_values(self) -> Self:
if linspace is not None and values is not None:
raise ValueError("'linspace' and 'values' are mutually exclusive)
return self
@property
def vals(self) -> npt.NDArray:
if self.values is not None:
return self.values
return np.linspace(*self.linspace) (Pydantic syntax, but trivial to translate in |
Personally I would be in favor of this.
Since having - id: resonator spectroscopy low power
operation: resonator_spectroscopy
targets: [0, 1]
parameters:
freq_width: { 0 : 5_000_000, 1: 4_000_000}
freq_step: { 0 : 100_000, 1: 200_000}
power_level: low
phase_sign: true
nshots: 1024 This is just a first attempt at a new interface. But I think that it would look much clearer compare to what we are doing now. |
It's not that simple, but we can keep the substance. E.g. while you're sweeping a pulse frequency, you almost never want to really specify the absolute value (~GHz) but always the offset (~MHz). And this is what is being done, simply, the drivers are deciding by themselves (each separately) which is the meaningful choice for each parameter. We could really convert it to absolute, but then Qibolab would only deal with ~GHz frequencies, and the conversion should be done inside Qibocal, fishing the base value from wherever it is. For the time being, there will only be QM around, so it's easy to decide what is the base type for each parameter. While adding the second driver (Zurich or Qblox), I'd enforce the default to be the same per parameter (possibly reintroducing the sweeper type only internally, not in the interface, and passing around this information). An interface for an actual sweeper type can be added, but truly later on.
I'm glad that there is room to improve even the Qibocal interface, keeping it optimal and still compatible. However, which will be the exact best Qibocal interface is a separate discussion. I would open an issue in Qibocal to discuss, as soon as we stabilize this (the attempt was useful to justify the Qibolab choice, but I'd not go any further). |
Thank you for the feedback. From what I understand we all agree to drop I am still not sure what is the expected behavior though. Originally the suggestion was to convert everything to
As far as I know, this is also how sweepers are currently used in qibocal, so in practice it is convenient to leave as they are (it will also be less work on the driver). I also agree that some of these options, such as Also, not related to sweeper types, but shall I drop |
I would be fine with doing the conversion in Qibocal. Especially if you consider non-runcard users this is already quite trivial to do, since we can just query the frequency in the python script.
I would say again that we should just have absolute values. When calibrating two qubit gates having amplitude sweeps done using factor can be painful.
We have some protocols in qibocal which are sweeping attenuation (for qblox) but I would hope that we can get rid of them soon-ish. I'm also happy to drop both
Yes, makes sense. |
If for Qibocal is better to have all of them as absolute, then let's do so. Even for Qibolab it's the easiest (just setting some values, no operation).
Regarding this, there might be some value in having them as sweepers, even software ones, but on the device (if available). It would save some networking.
Both are fine for me. Choose the most consistent one, according to your own taste. |
The main issue with I am also not sure to what extent we can sweep them even on device (even if not real time). I think QM provides a way to do so, but I have never tried it.
Same as the above, none of the existing configs has a |
The OPX and the Octave have an API for setting (the If you have a different idea, it would be quite interesting. But it's an optimization anyhow, so let's keep it simple, and leave the Python loops to Qibocal
Ah, right. So yes, I perfectly agree it should be |
|
I think that it could be useful to make use of
SweeperType
introduced in #459 for all drivers. In this way we could avoid to define global convention for each parameter that we sweep.I know that the
rfsoc
driver already supports this while in qblox this is currently being implemented in #518.@Jacfomg @stavros11 could you please port this feature also in QM and Zurich?
The text was updated successfully, but these errors were encountered: