qiboteam / qibolab

Quantum hardware module and drivers for Qibo.
https://qibo.science
Apache License 2.0
41 stars 10 forks source link

[WIP] Refactoring channels #792

Closed hay-k closed 3 months ago

hay-k commented 5 months ago

This proposal is aiming to bring qibolab closer to a future where it has a two-layer internal structure. The bottom layer handles instruments and does not know anything about quantum computing (does not know what gates, qubits, qubit pairs, etc. are; does not know about concepts like native gates, fast reset, etc.). It only knows how to generate signals into wires and acquire data from wires. It is not intended to be used by end users directly. The top layer knows necessary things about quantum computing, but nothing about instruments. It provides the necessary API for end users and is intended to be used by end users directly.

Part 1

The concept of a Channel already exists in qibolab. Here is a proposal on how to evolve this concept:

* I believe the general consensus at this point is that frequency should be property of channel and not of a pulse. This means all pulses on a given channel have the exact same frequency, and sweeping over a frequency means sweeping the frequencies of all pulses in that channel. However, none of this seems to break any existing functionality.

Part 2

Changes to Pulse and PulseSequence:

Part 3

Sweeping of pulse parameters can be largely left unchanged for now, but we have to implement sweeping over channel properties. In the initial implementation at least the frequency sweeps should work. On instrument level this is mostly rearranging existing code, however on the end-user API level we do not have any structure that can be used to represent such sweeps. Previously a frequency sweep was a pulse sweep.

TODO: open a separate proposal for this one.

alecandido commented 5 months ago

Part 1

The concept of a Channel already exists in qibolab. Here is a proposal on how to evolve this concept:

  • Channel shall become an abstract software level concept, and instances of channels do not necessarily have one-to-one mapping to physical wires, QPU connectors, or instrument ports. E.g. for setups with cross-resonance gates, each qubit will have two drive channels, 1. normal drive at its own frequency, 2. drive at the other qubit's frequency needed for CNOT. Physically, they could be two wires originating from different instruments that get multiplexed and connected to the physical drive of the qubit.

Agreed, I'm on the same page.

  • Channels will have unique names (I believe they already do, but we will need to change the way they are named). Experiments or circuits are described in terms of channel names - "I want to play this pulse on this channel", "I want to acquire data from that channel", etc.

Even fine, but here I'd like to understand if the word unique is a soft requirement to the user (UB otherwise), or whether we should enforce that.

  • There will be various types of channels, e.g. DCChannel, IQChannel, AcquireChannel, etc. The bottom layer of qibolab will provide instrument-specific implementations of channels (e.g. ZIIQChannel, ZIDCChannel, etc.).

I favor Enums for these kinds of things, rather than classes. Since a while, I try to use functions when I have to represent actions (doing something) and classes to collect data (being something). But well, that's related to the implementation. The concept of having [DC, IQ, Acquire] and [ZI, QM, Qblox, QICK] is perfectly fine.

  • For each quantum computer setup, there will be file that describes all available named channels. Currently this is done by Platform, but it also does a lot of other things like functionality for generating native gates/pulses. We most likely need to split Platform into smaller parts. One serves as glue between the two layers of qibolab by defining all available channels, and is not directly used by end-users. The other holds the remaining functionality of current Platform unless it is refactored as well.

For the time being, I'd keep the single entry point scheme, of a function constructing a platform. From the create() function itself, you could split your runcard in as many files as you wish, and we encourage splitting the parameters providing helper functions for serialization in Qibolab. A simpler interface is a more solid interface (though there are advantages in structuring it more, but I'd say it's not yet the time for that). The idea of an executable configuration was born exactly to define many channels, without reinventing our own language. The idea is to incrementally provide helper functions that are doing more and more for you, until the point in which you could even serialize the input (if ever, we may even keep the executable forever).

  • Besides channel names, end users will get access to some common (i.e. not instrument-specific) properties of channels (ChannelConfig). It will be possible to sweep and calibrate these properties. Some things that are currently properties of Pulse and Qubit classes, are actually channel properties, so they will be moved to respective ChannelConfig classes, e.g. frequency*, mixer calibration details, etc.

* I believe the general consensus at this point is that frequency should be property of channel and not of a pulse. This means all pulses on a given channel have the exact same frequency, and sweeping over a frequency means sweeping the frequencies of all pulses in that channel. However, none of this seems to break any existing functionality.

The part about sweeping frequencies is perfect. If you don't want to link the frequencies of some pulses, you will make even more logical channels, and sweep only one of them.

What is the advantage of having a separate ChannelConfig that is the same for everyone, instead of having them in a base class?

alecandido commented 5 months ago

I would discuss Part 3 somewhere else. If there is no issue, yet, that is fitting this topic, maybe you could open a dedicated one.

alecandido commented 5 months ago

Part 2

Changes to Pulse and PulseSequence:

  • Pulse will no longer have frequency property as mentioned above.
  • Pulse will no longer have properties type, channel, and qubit. These properties are responsible for describing the pulse embeding/position in a pulse sequence (along with start, but that one is already being removed in #789). So, some other party still needs to take care of this information.

Agreed, 100%.

  • PulseSequence will be that party. It will evolve from a simple list to at least a dict[str, list] mapping channel name to list of pulses.

This is the external layout. Even fine by me, I'm not strongly opinionated about this or its internal alternative. It will be an internal detail of Sequence and Pulse, we could revert this with little effort in the code (though it would require a breaking change, since part of the UI). I'd rather make the mapping dict[Channel, list[Pulse]] (i.e. str -> Channel), using (at least) a more telling type. But it's a detail.

However, you didn't stress in the previous nor in this point, but type and qubit should be conveyed through the channel. I believe we all agree about this point.

  • PulseSequence will allow various append functionality. E.g. append a pulse to a channel. E.g. append another sequence to a subset of channel, by taking care of mutual synchronization. Will be useful especially for two-qubit gates, which are not usually represented as a single pulse, but rather multiple synchronized pulses over multiple channels. There is no need for an insert functionality.

I'd rather keep the seq[ch].append(pulse) as the only core interface, and distinguish everything else as "high-level". You could use it, but it's definitely optional, and not more flexible than the other.

hay-k commented 5 months ago

I'd like to understand if the word unique is a soft requirement to the user (UB otherwise), or whether we should enforce that

Uniqueness here refers to a platform, i.e. channels can be identified by name without ambiguity. For user, the only requirement is to use names that exist in platform. Well, even that is not a requirement - they will just get an error if they use a channel name in pulse sequence that does not exist in platform. But to make user life (plus the life of the party who writes the platform) easier, there is a function that generates channel names, so they both can use it. It is part of the top layer.

I favor Enums for these kinds of things, rather than classes. Since a while, I try to use functions when I have to represent actions (doing something) and classes to collect data (being something).

These DCChannel, IQChannel, AcquireChannel are not containers of data, they are supposed to be abstract types, that define an interface. Then ZI, QM, etc. implement this interface. Such interface is needed so that we can extract common functionality from instrument-specific code and implement in a unified way (e.g. unrolling-batching).

For the time being, I'd keep the single entry point scheme, of a function constructing a platform.

We can keep the Platform in a single place for now, splitting is not critical at this stage. We can do something about it later. However, what I wanted to emphasize here, is that some things that the platform does are not end user's business, so it is better to have them somewhere else.

What is the advantage of having a separate ChannelConfig that is the same for everyone, instead of having them in a base class?

I am not sure I correctly understood this one. Do you mean why all users have to provide ChannelConfigs for each channel they use, instead of having channel configs as part of platform, and use them automatically? I have the feeling that this is not what you meant :D

I'd rather make the mapping dict[Channel, list[Pulse]] (i.e. str -> Channel), using (at least) a more telling type. But it's a detail.

yeah, I intended it to be NamedChannel (or, more precisely, whatever it ends up being as discussed in the other comment). Is this what you had in mind, or did you mean the key in the dict should be a more specialized channel object, like IQChannel?

However, you didn't stress in the previous nor in this point, but type and qubit should be conveyed through the channel. I believe we all agree about this point.

Well, according to the current state of the proposal, channels,

I would discuss Part 3 somewhere else. If there is no issue, yet, that is fitting this topic, maybe you could open a dedicated one.

Sure, as I mentioned I am planning to do this separately. This is not entirely a separate thing, because without it we are losing frequency sweeping functionality, but from organizational point of view we can implement and merge it separately as long as we are on this feature branch that is not released yet.

hay-k commented 5 months ago

@alecandido Thanks for the comments. Tagging @stavros11 as well to get some more attention. I forgot to tag you both in the original description.

hay-k commented 3 months ago

Closing this one. Another, more concrete PR will be submitted soon.