-
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
QM duration sweeper using multiple waveforms #979
Changes from 17 commits
c287813
b348c85
fb34c91
49b2a96
21f2eeb
eea337b
66a078f
0c84474
427f48d
431ed6e
b4fc45c
46d1a8b
704ed63
36cdf08
b0b1227
2fffca7
433a2c2
4e4c2a5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 |
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -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): | ||||
qua.play(sweep_op, element) | ||||
|
||||
|
||||
def _play( | ||||
|
@@ -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) | ||||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The set is not the problem, but rather
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... There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 list[tuple[Union[ChannelId, set[ChannelId]], PulseLike]] It isn't much simpler, it's just that the second tuple element is always My issue with all these solutions is that, although they are more efficient when There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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. |
||||
|
||||
for channel_id, pulse in args.sequence: | ||||
element = str(channel_id) | ||||
op = operation(pulse) | ||||
|
@@ -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) | ||||
|
@@ -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 |
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?There was a problem hiding this comment.
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
incase_
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), becausequa.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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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).