Skip to content
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

QM duration sweeper using multiple waveforms #979

Merged
merged 18 commits into from
Aug 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 54 additions & 22 deletions src/qibolab/instruments/qm/controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,18 @@
from qm.simulate.credentials import create_credentials
from qualang_tools.simulator_tools import create_simulator_controller_connections

from qibolab.components import AcquireChannel, Config, DcChannel, IqChannel
from qibolab.components import AcquireChannel, Channel, Config, DcChannel, IqChannel
from qibolab.execution_parameters import ExecutionParameters
from qibolab.identifier import ChannelId
from qibolab.instruments.abstract import Controller
from qibolab.pulses.pulse import Acquisition, Delay, VirtualZ, _Readout
from qibolab.pulses.pulse import Acquisition, Align, Delay, Pulse, _Readout
from qibolab.sequence import PulseSequence
from qibolab.sweeper import ParallelSweepers, Parameter
from qibolab.sweeper import ParallelSweepers, Parameter, Sweeper
from qibolab.unrolling import Bounds

from .components import QmChannel
from .config import SAMPLING_RATE, QmConfig, operation
from .program import create_acquisition, program
from .program import ExecutionArguments, create_acquisition, program

OCTAVE_ADDRESS_OFFSET = 11000
"""Offset to be added to Octave addresses, because they must be 11xxx, where
Expand Down Expand Up @@ -112,6 +112,11 @@ def fetch_results(result, acquisitions):
}


def find_duration_sweepers(sweepers: list[ParallelSweepers]) -> list[Sweeper]:
"""Find duration sweepers in order to register multiple pulses."""
return [s for ps in sweepers for s in ps if s.parameter is Parameter.duration]


@dataclass
class QmController(Controller):
""":class:`qibolab.instruments.abstract.Controller` object for controlling
Expand Down Expand Up @@ -291,6 +296,24 @@ def configure_channels(
channel = self.channels[str(channel_id)]
self.configure_channel(channel, configs)

def register_pulse(self, channel: Channel, pulse: Pulse) -> str:
"""Add pulse in the QM ``config`` and return corresponding
operation."""
# if (
# pulse.duration % 4 != 0
# or pulse.duration < 16
# or pulse.id in pulses_to_bake
# ):
# qmpulse = BakedPulse(pulse, element)
# qmpulse.bake(self.config, durations=[pulse.duration])
# else:
name = str(channel.name)
if isinstance(channel, DcChannel):
return self.config.register_dc_pulse(name, pulse)
if channel.acquisition is None:
return self.config.register_iq_pulse(name, pulse)
return self.config.register_acquisition_pulse(name, pulse)

def register_pulses(self, configs: dict[str, Config], sequence: PulseSequence):
"""Adds all pulses except measurements of a given sequence in the QM
``config``.
Expand All @@ -299,21 +322,26 @@ def register_pulses(self, configs: dict[str, Config], sequence: PulseSequence):
acquisitions (dict): Map from measurement instructions to acquisition objects.
"""
for channel_id, pulse in sequence:
if not isinstance(pulse, (Acquisition, Delay, VirtualZ)):
name = str(channel_id)
channel = self.channels[name].logical_channel
# if (
# pulse.duration % 4 != 0
# or pulse.duration < 16
# or pulse.id in pulses_to_bake
# ):
# qmpulse = BakedPulse(pulse, element)
# qmpulse.bake(self.config, durations=[pulse.duration])
# else:
if isinstance(channel, DcChannel):
self.config.register_dc_pulse(name, pulse)
elif channel.acquisition is None:
self.config.register_iq_pulse(name, pulse)
if isinstance(pulse, Pulse):
channel = self.channels[str(channel_id)].logical_channel
self.register_pulse(channel, pulse)

def register_duration_sweeper_pulses(
self, args: ExecutionArguments, sweeper: Sweeper
):
"""Register pulse with many different durations, in order to sweep
duration."""
for pulse in sweeper.pulses:
if isinstance(pulse, (Align, Delay)):
continue

op = operation(pulse)
channel_name = str(args.sequence.pulse_channels(pulse.id)[0])
channel = self.channels[channel_name].logical_channel
for value in sweeper.values:
sweep_pulse = pulse.model_copy(update={"duration": value})
sweep_op = self.register_pulse(channel, sweep_pulse)
args.parameters[op].pulses.append((value, sweep_op))

def register_acquisitions(
self,
Expand Down Expand Up @@ -395,7 +423,13 @@ def play(
self.configure_channels(configs, sequence.channels)
self.register_pulses(configs, sequence)
acquisitions = self.register_acquisitions(configs, sequence, options)
experiment = program(configs, sequence, options, acquisitions, sweepers)

args = ExecutionArguments(sequence, acquisitions, options.relaxation_time)

for sweeper in find_duration_sweepers(sweepers):
self.register_duration_sweeper_pulses(args, sweeper)

experiment = program(configs, args, options, sweepers)

if self.manager is None:
warnings.warn(
Expand All @@ -405,8 +439,6 @@ def play(

if self.script_file_name is not None:
script = generate_qua_script(experiment, asdict(self.config))
for _, pulse in sequence:
script = script.replace(operation(pulse), str(pulse))
with open(self.script_file_name, "w") as file:
file.write(script)

Expand Down
1 change: 1 addition & 0 deletions src/qibolab/instruments/qm/program/__init__.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
from .acquisition import Acquisitions, create_acquisition
from .arguments import ExecutionArguments
from .instructions import program
5 changes: 3 additions & 2 deletions src/qibolab/instruments/qm/program/arguments.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from qibolab.sequence import PulseSequence

from .acquisition import Acquisition
from .acquisition import Acquisitions


@dataclass
Expand All @@ -16,6 +16,7 @@ class Parameters:
duration: Optional[_Variable] = None
amplitude: Optional[_Variable] = None
phase: Optional[_Variable] = None
pulses: list[tuple[float, str]] = field(default_factory=list)


@dataclass
Expand All @@ -27,7 +28,7 @@ class ExecutionArguments:
"""

sequence: PulseSequence
acquisitions: dict[tuple[str, str], Acquisition]
acquisitions: Acquisitions
relaxation_time: int = 0
parameters: dict[str, Parameters] = field(
default_factory=lambda: defaultdict(Parameters)
Expand Down
48 changes: 34 additions & 14 deletions src/qibolab/instruments/qm/program/instructions.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,30 @@

from qibolab.components import Config
from qibolab.execution_parameters import AcquisitionType, ExecutionParameters
from qibolab.pulses import Delay, Pulse
from qibolab.sequence import PulseSequence
from qibolab.pulses import Align, Delay, Pulse, VirtualZ
from qibolab.sweeper import ParallelSweepers

from ..config import operation
from .acquisition import Acquisition, Acquisitions
from .acquisition import Acquisition
from .arguments import ExecutionArguments, Parameters
from .sweepers import INT_TYPE, NORMALIZERS, SWEEPER_METHODS
from .sweepers import INT_TYPE, NORMALIZERS, SWEEPER_METHODS, normalize_phase


def _delay(pulse: Delay, element: str, parameters: Parameters):
# TODO: How to play delays on multiple elements?
if parameters.duration is None:
duration = int(pulse.duration) // 4 + 1
duration = int(pulse.duration) // 4
else:
duration = parameters.duration
qua.wait(duration, element)
qua.wait(duration + 1, element)


def _play_multiple_waveforms(element: str, parameters: Parameters):
"""Sweeping pulse duration using distinctly uploaded waveforms."""
with qua.switch_(parameters.duration, unsafe=True):
for value, sweep_op in parameters.pulses:
with qua.case_(value // 4):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is to avoid baking. What happens for the values covered multiple times?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And isn't there a // 4 already in the normalizing function?

Copy link
Member Author

@stavros11 stavros11 Aug 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now this is not related to baking, but it will most likely need to be modified when baking is implemented.

Basically, the reason we need the // 4 in case_ is because it also exists in the normalizing function. In particular, the loop variable (parameters.duration) will take values from an array that was normalized with // 4, while the uploaded waveforms are enumerated with the original array, so we have to divide the enumerator when we do the matching, in order to properly match with the loop variable. We could drop the normalization in both places, however then we would need to make sure to normalize when delays are involved in the same sweeper (eg. Rabi length sweeps the duration of drive pulse and readout delay with the same sweeper), because qua.wait expects clock cycles (=4ns).

Anyway, this will probably change when baking (=supporting pulses with 1ns resolution, the best we can currently do) is implemented, so I would postpone until then, as I am planning to do it right after this PR anyway, together with the other issues from #969. Otherwise, I could push baking directly here, as I don't expect it to be very complicated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so I get that if you're receiving as values [4, 5, 6, 7] you'll sweep four elements, but the value will always be the same one.

For as long as it is documented, for me it's perfectly acceptable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, these values are not supported yet since baking is not implemented, so only waveforms with length that is multiple of 4 are supported (and >16). I am not sure what will be the exact behavior as I haven't tried, but I would guess that an error will appear if we try to upload a waveform of these lengths. In any case, I will implement this before we release 0.2 so it should be fine. For now I added a reference to this discussion in #969.

One additional point to check is what happens to the interpolated sweep in this case. I guess for these lengths it will fail because they are very short, however for something like [20, 21, 22, 23] it may not fail and actually return four overlapping points. If that happens we may want to raise an error ourselves and prompt to use non-interpolated sweep (which will support baking).

qua.play(sweep_op, element)


def _play(
Expand All @@ -36,10 +43,13 @@ def _play(
if parameters.amplitude is not None:
op = op * parameters.amplitude

if acquisition is not None:
acquisition.measure(op)
if len(parameters.pulses) > 0:
_play_multiple_waveforms(element, parameters)
else:
qua.play(op, element, duration=parameters.duration)
if acquisition is not None:
acquisition.measure(op)
else:
qua.play(op, element, duration=parameters.duration)

if parameters.phase is not None:
qua.reset_frame(element)
Expand All @@ -51,6 +61,12 @@ def play(args: ExecutionArguments):
Should be used inside a ``program()`` context.
"""
qua.align()

# keep track of ``Align`` command that were already played
# because the same ``Align`` will appear on multiple channels
# in the sequence
processed_aligns = set()
Comment on lines +65 to +68
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether at this level it is just simpler to preprocess the sequence, and replace Align by a plain list of channels, while replacing the channel e.g. with an empty string, or the "align" string. And then, if channel_id == "align" you do qua.align(*pulse).
It's not that elegant from the point of view of the types (though it could be made more elegant - but it's not needed), still it should be much simpler (and you avoid iterating the whole sequence every time that you encounter an align, also because you know they are supposed to be consecutive, otherwise it would be a bug).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not that elegant from the point of view of the types (though it could be made more elegant - but it's not needed), still it should be much simpler (and you avoid iterating the whole sequence every time that you encounter an align, also because you know they are supposed to be consecutive, otherwise it would be a bug).

Indeed, my main issue with this is that it is not that elegant and may require more code from the driver side, like having a QM internal sequence, while I am trying to rely on qibolab primitives as much as possible. Also, the solution with the set shouldn't be much overhead in terms of performance and it is also more localized, just a few additional lines of code in a single place. The preprocess would probably have to happen somewhere else and copy the whole sequence, just to handle Align which is a very special and potentially rare case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The set is not the problem, but rather

channel_ids = args.sequence.pulse_channels(pulse.id)

which requires iterating the whole sequence for every Align (for each ID, not each combination (ch, Align), of course).

I was not proposing a separate sequence, just a single (inelegant) function doing:

def process_aligns(sequence: PulseSequence) -> list[Union[tuple[ChannelId, PulseLike], tuple[Literal["align"], set[ChannelId]]]]:
    new = []
    align_channels = set()
    def align():
        if len(align_channels) > 0:
            new.append(("align", align_channels))
            align_channels.clear()
    for (ch, p) in sequence:
        if isinstance(p, Align):
            align_channels.add(ch)
        else:
            align()
            new.append((ch, p))
    align()
    return new

You can tell it's not elegant from the return type (though it could be written better than that), but it requires a single iteration, without any QM-specific object (it could even be part of the sequence API...).

However, as I said, it's negligible. I'm just obsessed...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks for the clarification. That is fine with me and I could add it to the code. Maybe a simplification would be to use the actual Align object in the tuple, instead of Literal["align"] and have the following return type:

list[tuple[Union[ChannelId, set[ChannelId]], PulseLike]]

It isn't much simpler, it's just that the second tuple element is always PulseLike. We could even get rid of the Union if we convert the first element to be a list or set always, with single element for non-aligns.

My issue with all these solutions is that, although they are more efficient when Aligns are present, they are slightly less efficient when Aligns are not present. For example in this case we'd have to copy the sequence even when Align is not present. We could check and only call process_aligns if they are actually present, but even this check would be worst case O(n) (n=len(sequence)) since we'd have to check the whole sequence. In summary, the proposed solution would be O(2n) in all cases, while the current implementation is O((m + 1)n) (m=number of aligns), so if this is correct it depends on what we expect more often. For example the current compiler doesn't use Align (and most qibocal routines will not either) so if we optimize for circuits the current implementation may actually be better in most cases. Of course negligibly better, but since we are discussing this anyway...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, in practice the trade-off is even less clear, because the operations involved are not all equivalent: in one case you're just copying some Align (though iterating more PulseLike, if many Align are present), but the other is possibly doing fewer operations, but always copying the whole sequence. So, if copying were much more expensive than iterating, we had a winner...

In any case, as I said, it's just for the sake of discussion. The truth is that this part is not the performance bottleneck.
(already loading the whole platform, creating many objects, and parsing the whole parameters.json, irrespectively of the sequence, should be much more relevant - and still negligible, since most of the execution time in current experiments will be always the actual runtime on the device, and then the networking required)


for channel_id, pulse in args.sequence:
element = str(channel_id)
op = operation(pulse)
Expand All @@ -60,6 +76,12 @@ def play(args: ExecutionArguments):
elif isinstance(pulse, Pulse):
acquisition = args.acquisitions.get((op, element))
_play(op, element, params, acquisition)
elif isinstance(pulse, VirtualZ):
qua.frame_rotation_2pi(normalize_phase(pulse.phase), element)
elif isinstance(pulse, Align) and pulse.id not in processed_aligns:
channel_ids = args.sequence.pulse_channels(pulse.id)
qua.align(*(str(ch) for ch in channel_ids))
processed_aligns.add(pulse.id)

if args.relaxation_time > 0:
qua.wait(args.relaxation_time // 4)
Expand Down Expand Up @@ -110,25 +132,23 @@ def sweep(

def program(
configs: dict[str, Config],
sequence: PulseSequence,
args: ExecutionArguments,
options: ExecutionParameters,
acquisitions: Acquisitions,
sweepers: list[ParallelSweepers],
):
"""QUA program implementing the required experiment."""
with qua.program() as experiment:
n = declare(int)
# declare acquisition variables
for acquisition in acquisitions.values():
for acquisition in args.acquisitions.values():
acquisition.declare()
# execute pulses
args = ExecutionArguments(sequence, acquisitions, options.relaxation_time)
with for_(n, 0, n < options.nshots, n + 1):
sweep(list(sweepers), configs, args)
# download acquisitions
has_iq = options.acquisition_type is AcquisitionType.INTEGRATION
buffer_dims = options.results_shape(sweepers)[::-1][int(has_iq) :]
with qua.stream_processing():
for acquisition in acquisitions.values():
for acquisition in args.acquisitions.values():
acquisition.download(*buffer_dims)
return experiment
18 changes: 15 additions & 3 deletions src/qibolab/instruments/qm/program/sweepers.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,15 +150,26 @@ def _duration(
args.parameters[operation(pulse)].duration = variable


INT_TYPE = {Parameter.frequency, Parameter.duration}
def normalize_phase(values):
"""Normalize phase from [0, 2pi] to [0, 1]."""
return values / (2 * np.pi)


def normalize_duration(values):
"""Convert duration from ns to clock cycles (clock cycle = 4ns)."""
return (values // 4).astype(int)


INT_TYPE = {Parameter.frequency, Parameter.duration, Parameter.duration_interpolated}
"""Sweeper parameters for which we need ``int`` variable type.

The rest parameters need ``fixed`` type.
"""

NORMALIZERS = {
Parameter.relative_phase: lambda values: values / (2 * np.pi),
Parameter.duration: lambda values: (values // 4).astype(int),
Parameter.relative_phase: normalize_phase,
Parameter.duration: normalize_duration,
Parameter.duration_interpolated: normalize_duration,
}
"""Functions to normalize sweeper values.

Expand All @@ -169,6 +180,7 @@ def _duration(
Parameter.frequency: _frequency,
Parameter.amplitude: _amplitude,
Parameter.duration: _duration,
Parameter.duration_interpolated: _duration,
Parameter.relative_phase: _relative_phase,
Parameter.bias: _bias,
}
Expand Down
2 changes: 1 addition & 1 deletion src/qibolab/pulses/__init__.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
from .envelope import *
from .pulse import Acquisition, Delay, Pulse, PulseLike, VirtualZ
from .pulse import Acquisition, Align, Delay, Pulse, PulseLike, VirtualZ
2 changes: 2 additions & 0 deletions src/qibolab/pulses/plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ def sequence(ps: PulseSequence, freq: dict[str, float], filename=None):
import matplotlib.pyplot as plt
from matplotlib import gridspec

# compile ``Align`` to delays as it is not supported here
ps = ps.align_to_delays()
num_pulses = len(ps)
_ = plt.figure(figsize=(14, 2 * num_pulses), dpi=200)
gs = gridspec.GridSpec(ncols=1, nrows=num_pulses)
Expand Down
9 changes: 8 additions & 1 deletion src/qibolab/pulses/pulse.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,13 @@ def id(self) -> int:
return self.acquisition.id


class Align(_PulseLike):
"""Brings different channels at the same point in time."""

kind: Literal["align"] = "align"


PulseLike = Annotated[
Union[Pulse, Delay, VirtualZ, Acquisition, _Readout], Field(discriminator="kind")
Union[Align, Pulse, Delay, VirtualZ, Acquisition, _Readout],
Field(discriminator="kind"),
]
Loading