Qblox

[alecandido]

This will be the prototype for the 0.2 Qblox driver, based on the Q1ASM handling layed out in #868

[codecov]

Codecov Report

Attention: Patch coverage is 5.11364% with 167 lines in your changes missing coverage. Please review.

Project coverage is 51.69%. Comparing base (f7a8883) to head (4a14942).

Files with missing lines	Patch %	Lines
src/qibolab/_core/instruments/qblox/cluster.py	0.00%	96 Missing ⚠️
src/qibolab/_core/instruments/qblox/platform.py	0.00%	43 Missing ⚠️
src/qibolab/_core/instruments/qblox/sequence.py	0.00%	24 Missing ⚠️
src/qibolab/instruments/qblox.py	0.00%	4 Missing ⚠️

Additional details and impacted files

@@               Coverage Diff               @@
##           parse-q1asm    #1088      +/-   ##
===============================================
- Coverage        54.35%   51.69%   -2.67%     
===============================================
  Files               66       70       +4     
  Lines             3170     3341     +171     
===============================================
+ Hits              1723     1727       +4     
- Misses            1447     1614     +167     

Flag	Coverage Δ
unittests	51.69% <5.11%> (-2.67%)	⬇️

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.

[alecandido]

@stavros11 I start wondering why do we have this distinction at all:

qibolab/src/qibolab/_core/components/channels.py

Lines 32 to 35 in f74c334

	device: str = ""
	"""Name of the device."""
	path: str = ""
	"""Physical port addresss within the device."""

Couldn't we have just a path attribute, which is parsed by every instrument in its own way?

It would have made sense if Channel were only part of the Platform, and we had to attribute the instances to the various instruments, and thus device could have served that purpose, matching the name of the instrument in the Platform.instruments dictionary.
But Channels are already part of the Controller class itself, so we don't need to perform this operation at all...

Moreover, we internally distinguish channels only by their logical name (stemming from Qubit's attributes), while this physical layer is fully internal to the driver.

If we agree on this, what we could do is to deprecate .device, making it Optional and assigning a default None value.
Formally, this would break the interface (as there may be functions assuming that attribute to always be a string, and never None). In practice, it should not break anything.

The other option is to just deprecate it "among us", and open an issue for removal in 0.3. Since it already has a default value (empty string), we just ignore it in the drivers and that's it (which is what I'm planning to do right now).

[stavros11]

Couldn't we have just a path attribute, which is parsed by every instrument in its own way?

Yes, this should be possible. I guess the distinction is leftover from 0.1 and maybe was relevant up to some point in 0.2, but it is not anymore.

If we agree on this, what we could do is to deprecate .device, making it Optional and assigning a default None value. Formally, this would break the interface (as there may be functions assuming that attribute to always be a string, and never None). In practice, it should not break anything.

I think it will break the QM driver, as channel.device is used in a few places and using None there would probably lead to failure. It should be easy to update this to read the device from the path, however if we want to keep our promise and not break the public interface within 0.2 versions, we should also maintain the use of device for QM. Otherwise the current platforms of qibolab_platforms_qrc main which use device will fail until we change them. I should be able to do something like

if channel.device is None:
    device = # parse from `channel.path`
else:
    device = channel.device
...

It is not optimal, as I generally don't like providing two (equally trivial) interfaces to do the same thing, but I would be fine doing it only for QM, if it will simplify all other drivers.

For other drivers, we can completly ignore device and use only path and in principle we are not breaking anything as these drivers never existed in the first place. Also, I think Keysight will not be affected by this.

[alecandido]

Ah, yes, the idea was to absolutely do something like if channel.device is None, but the idea is that, in principle, even that would be breaking.

E.g. if you have a function that takes a Channel and does channel.device.startswith() that would be broken, since it relied on channel.device always being a string.

In practice, I don't expect that there is any function like that (nor anyone using Channel content beyond the drivers, which are internal).
However, let me take that proposal back: in this case, an empty string should be as good as None, and it's already there (even as default).

[stavros11]

I know this is WIP and I am not sure exactly how it is used, but I would expect drivers to be qubit/coupler agnostic and only play with channels and configs, without caring exactly which qubit each channel controls, but maybe only the channel type.

[alecandido]

This is just a support function for writing "simpler" platforms, which I moved from the current sketch of the platform.

The idea is that using this function is fully optional. But the function itself is opinionated, favoring certain layouts (hopefully, the most frequent ones), making it simpler to describe the connections.
An example of its usage is here:
https://github.com/qiboteam/qibolab_platforms_qrc/blob/e4011ded804135d758310082616860bb959d94fd/iqm5q/platform.py#L16-L27
(you can see that this is attempting to mirror the connections that you physically have in the lab, to make the definition simpler when you have that picture - the function is supposed to do the rest - see conventions and caveats in its docstring)

To be fair, the reason why I introduced this is that we're passing qubits and couplers separately in the platform, and consequently defining them in two separate sequences.
However, I refactored many times to improve the representation and implementation, and by now there is a single relic for that separation, i.e.:

qibolab/src/qibolab/_core/instruments/qblox/platform.py

Lines 93 to 96 in 1514f00

	if kind == "qubits":
	channels[el.acquisition] = channels[el.acquisition].model_copy(
	update={"probe": el.probe}
	)

All the rest is already performed in the very same way.
So, since it is just post-processing, I should be able to concatenate qubits and couplers, and just post-process qubits for the acquisition.

[stavros11]

I have not looked at the full content of the file in detail so maybe it is indeed useful, however my main concern (and what I don't understand) is why this is under qblox. I would expect the instrument to have no knowledge about qubits and couplers at all, as the Platform never communicates these objects to the instrument.

If it is actually useful, it would make more sense to lift this outside the drivers and make it usable by any platform.

[alecandido]

As said above, these qubits & couplers issue I should be able to solve, and I will (try to) do it soon.

If it is actually useful, it would make more sense to lift this outside the drivers and make it usable by any platform.

We can consider generalizing it, with some pluggable choices. But for the time being it is tailored to Qblox in a few ways (not infinitely many).

The most outstanding example:

qibolab/src/qibolab/_core/instruments/qblox/platform.py

Lines 19 to 23 in 1514f00

	if mod.startswith("qcm_rf"):
	return "drive", IqChannel
	if mod.startswith("qcm"):
	return "flux", DcChannel
	if mod.startswith("qrm_rf"):

true that we could provide some mappings from module names. But not even all instruments have modules... (e.g. not QICK)

Let's say that I'm using Qblox to experiment the approach. But I'm not aiming to make something more general than that, at first shot.
And keeping it as a fully optional part, even for Qblox, we should be free to just leave it there at some point, and replace with some more general implementation (if general-enough, by reimplementing the current interface making use of the new one, otherwise by keeping both, and deprecating the Qblox-specific one).

[alecandido]

@aorgazf @DavidSarlle apparently Qblox sequencers can be connected to be used for input, output, or both https://docs.qblox.com/en/main/api_reference/cluster.html#qblox_instruments.Cluster.connect_sequencer

I'm taking into account both the input and output case (of course), but I don't see how the io (aka both) option may be used at all...
I.e. the sequencer is attached to one or two physical channels, that are either input channels or output channels (obviously not both...). And even if it's attached to two channels, it's only to work in complex mode (I & Q).

Do you have any use case for this io mode? Is there anything evident I'm missing?
(I will also ask explanation to the support team, but I don't consider this urgent...)

[DavidSarlle]

@aorgazf @DavidSarlle apparently Qblox sequencers can be connected to be used for input, output, or both https://docs.qblox.com/en/main/api_reference/cluster.html#qblox_instruments.Cluster.connect_sequencer

I'm taking into account both the input and output case (of course), but I don't see how the io (aka both) option may be used at all... I.e. the sequencer is attached to one or two physical channels, that are either input channels or output channels (obviously not both...). And even if it's attached to two channels, it's only to work in complex mode (I & Q).

Do you have any use case for this io mode? Is there anything evident I'm missing? (I will also ask explanation to the support team, but I don't consider this urgent...)

Sorry for the late rely @alecandido. I have not seen before thi possibility before. And as you said I can not see when we can use the channel in both ways. But @aorgazf knows better than me the driver and he has some explanation or example where ports it can be used in that mode.