Migrate Pulse to dataclass

[alecandido]

First batch of #683.

Since these updates are going to affect many places, the idea is to attempt bundling changes in a few units, and make sure everything is still working every time (also to avoid a single huge PR).

Checklist:

• [ ] Reviewers confirm new code works as expected.
• [ ] Tests are passing.
• [ ] Coverage does not decrease.
• [ ] Documentation is updated.

[codecov[bot]]

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (da6d1b9) 63.58% compared to head (e6168d4) 62.94%.

Files	Patch %	Lines
src/qibolab/pulses.py	89.79%	5 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #709 +/- ## ========================================== - Coverage 63.58% 62.94% -0.65% ========================================== Files 49 49 Lines 6539 6399 -140 ========================================== - Hits 4158 4028 -130 + Misses 2381 2371 -10 ``` | [Flag](https://app.codecov.io/gh/qiboteam/qibolab/pull/709/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/qiboteam/qibolab/pull/709/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam) | `62.94% <89.79%> (-0.65%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[alecandido]

@stavros11 @andrea-pasquale at this point I would actually also like to drop ._if: it is only used by Qblox

❯ rg '\._if'
tests/test_pulses.py
694:        2 * np.pi * pulse._if * pulse.start / 1e9
697:        i, q, num_samples, pulse._if, global_phase + pulse.relative_phase, sampling_rate
719:        == f"Modulated_Waveform_I(num_samples = {num_samples}, amplitude = {format(pulse.amplitude, '.6f').rstrip('0').rstrip('.')}, shape = {str(pulse.shape)}, frequency = {format(pulse._if, '_')}, phase = {format(global_phase + pulse.relative_phase, '.6f').rstrip('0').rstrip('.')})"
723:        == f"Modulated_Waveform_Q(num_samples = {num_samples}, amplitude = {format(pulse.amplitude, '.6f').rstrip('0').rstrip('.')}, shape = {str(pulse.shape)}, frequency = {format(pulse._if, '_')}, phase = {format(global_phase + pulse.relative_phase, '.6f').rstrip('0').rstrip('.')})"
764:        i, q, num_samples, pulse._if, global_phase + pulse.relative_phase, sampling_rate
786:        == f"Modulated_Waveform_I(num_samples = {num_samples}, amplitude = {format(pulse.amplitude, '.6f').rstrip('0').rstrip('.')}, shape = {str(pulse.shape)}, frequency = {format(pulse._if, '_')}, phase = {format(global_phase + pulse.relative_phase, '.6f').rstrip('0').rstrip('.')})"
790:        == f"Modulated_Waveform_Q(num_samples = {num_samples}, amplitude = {format(pulse.amplitude, '.6f').rstrip('0').rstrip('.')}, shape = {str(pulse.shape)}, frequency = {format(pulse._if, '_')}, phase = {format(global_phase + pulse.relative_phase, '.6f').rstrip('0').rstrip('.')})"
834:        2 * np.pi * pulse._if * pulse.start / 1e9
837:        i, q, num_samples, pulse._if, global_phase + pulse.relative_phase, sampling_rate
859:        == f"Modulated_Waveform_I(num_samples = {num_samples}, amplitude = {format(pulse.amplitude, '.6f').rstrip('0').rstrip('.')}, shape = {str(pulse.shape)}, frequency = {format(pulse._if, '_')}, phase = {format(global_phase + pulse.relative_phase, '.6f').rstrip('0').rstrip('.')})"
863:        == f"Modulated_Waveform_Q(num_samples = {num_samples}, amplitude = {format(pulse.amplitude, '.6f').rstrip('0').rstrip('.')}, shape = {str(pulse.shape)}, frequency = {format(pulse._if, '_')}, phase = {format(global_phase + pulse.relative_phase, '.6f').rstrip('0').rstrip('.')})"

src/qibolab/instruments/qblox/controller.py
183:                    pulse._if = int(pulse.frequency - pulse_channel.lo_frequency)

src/qibolab/pulses.py
168:        if abs(pulse._if) * 2 > sampling_rate:
176:            2 * np.pi * pulse._if * time + global_phase + pulse.relative_phase
179:            2 * np.pi * pulse._if * time + global_phase + pulse.relative_phase
200:        modulated_waveform_i.serial = f"Modulated_Waveform_I(num_samples = {num_samples}, amplitude = {format(pulse.amplitude, '.6f').rstrip('0').rstrip('.')}, shape = {str(pulse.shape)}, frequency = {format(pulse._if, '_')}, phase = {format(global_phase + pulse.relative_phase, '.6f').rstrip('0').rstrip('.')})"
202:        modulated_waveform_q.serial = f"Modulated_Waveform_Q(num_samples = {num_samples}, amplitude = {format(pulse.amplitude, '.6f').rstrip('0').rstrip('.')}, shape = {str(pulse.shape)}, frequency = {format(pulse._if, '_')}, phase = {format(global_phase + pulse.relative_phase, '.6f').rstrip('0').rstrip('.')})"

@PiergiorgioButtarini is it possible to store this information anywhere else in Qblox?

[andrea-pasquale]

@stavros11 @andrea-pasquale at this point I would actually also like to drop ._if: it is only used by Qblox

❯ rg '\._if'
tests/test_pulses.py
694:        2 * np.pi * pulse._if * pulse.start / 1e9
697:        i, q, num_samples, pulse._if, global_phase + pulse.relative_phase, sampling_rate
719:        == f"Modulated_Waveform_I(num_samples = {num_samples}, amplitude = {format(pulse.amplitude, '.6f').rstrip('0').rstrip('.')}, shape = {str(pulse.shape)}, frequency = {format(pulse._if, '_')}, phase = {format(global_phase + pulse.relative_phase, '.6f').rstrip('0').rstrip('.')})"
723:        == f"Modulated_Waveform_Q(num_samples = {num_samples}, amplitude = {format(pulse.amplitude, '.6f').rstrip('0').rstrip('.')}, shape = {str(pulse.shape)}, frequency = {format(pulse._if, '_')}, phase = {format(global_phase + pulse.relative_phase, '.6f').rstrip('0').rstrip('.')})"
764:        i, q, num_samples, pulse._if, global_phase + pulse.relative_phase, sampling_rate
786:        == f"Modulated_Waveform_I(num_samples = {num_samples}, amplitude = {format(pulse.amplitude, '.6f').rstrip('0').rstrip('.')}, shape = {str(pulse.shape)}, frequency = {format(pulse._if, '_')}, phase = {format(global_phase + pulse.relative_phase, '.6f').rstrip('0').rstrip('.')})"
790:        == f"Modulated_Waveform_Q(num_samples = {num_samples}, amplitude = {format(pulse.amplitude, '.6f').rstrip('0').rstrip('.')}, shape = {str(pulse.shape)}, frequency = {format(pulse._if, '_')}, phase = {format(global_phase + pulse.relative_phase, '.6f').rstrip('0').rstrip('.')})"
834:        2 * np.pi * pulse._if * pulse.start / 1e9
837:        i, q, num_samples, pulse._if, global_phase + pulse.relative_phase, sampling_rate
859:        == f"Modulated_Waveform_I(num_samples = {num_samples}, amplitude = {format(pulse.amplitude, '.6f').rstrip('0').rstrip('.')}, shape = {str(pulse.shape)}, frequency = {format(pulse._if, '_')}, phase = {format(global_phase + pulse.relative_phase, '.6f').rstrip('0').rstrip('.')})"
863:        == f"Modulated_Waveform_Q(num_samples = {num_samples}, amplitude = {format(pulse.amplitude, '.6f').rstrip('0').rstrip('.')}, shape = {str(pulse.shape)}, frequency = {format(pulse._if, '_')}, phase = {format(global_phase + pulse.relative_phase, '.6f').rstrip('0').rstrip('.')})"

src/qibolab/instruments/qblox/controller.py
183:                    pulse._if = int(pulse.frequency - pulse_channel.lo_frequency)

src/qibolab/pulses.py
168:        if abs(pulse._if) * 2 > sampling_rate:
176:            2 * np.pi * pulse._if * time + global_phase + pulse.relative_phase
179:            2 * np.pi * pulse._if * time + global_phase + pulse.relative_phase
200:        modulated_waveform_i.serial = f"Modulated_Waveform_I(num_samples = {num_samples}, amplitude = {format(pulse.amplitude, '.6f').rstrip('0').rstrip('.')}, shape = {str(pulse.shape)}, frequency = {format(pulse._if, '_')}, phase = {format(global_phase + pulse.relative_phase, '.6f').rstrip('0').rstrip('.')})"
202:        modulated_waveform_q.serial = f"Modulated_Waveform_Q(num_samples = {num_samples}, amplitude = {format(pulse.amplitude, '.6f').rstrip('0').rstrip('.')}, shape = {str(pulse.shape)}, frequency = {format(pulse._if, '_')}, phase = {format(global_phase + pulse.relative_phase, '.6f').rstrip('0').rstrip('.')})"

@PiergiorgioButtarini is it possible to store this information anywhere else in Qblox?

If it possible I would also drop it at this point. I believe that in theory the if_ is completely determined by the frequency of the pulse that we want to send and the lo frequency. I don't know exactly how the qblox driver works but if during the pulse execution it could access the frequency of the los I think it should be relatively easy to remove the if_ completely.

[alecandido]

I don't know exactly how the qblox driver works but if during the pulse execution it could access the frequency of the los I think it should be relatively easy to remove the if_ completely.

This I'm trying to understand myself. But, as you can see from the quoted line, Qblox is actually setting this information from the LO at some point, a point when the pulse is accessible (thus during an invocation of some kind of play). So, it is only used to save the information in the meanwhile. We just need to postpone the computation, and propagate the LO instead of the IF.

[alecandido]

Tested on Qblox, working ✅

Pytest report

``` alessandro.candido@saadiyat:~/qibolab$ srun -p qw5q_gold bash -c 'export QIBOLAB_PLATFORMS=$HOME/qibolab_platforms_qrc; poetry run pytest -m qpu --platform qw5q_gold' ============================= test session starts ============================== platform linux -- Python 3.10.12, pytest-7.4.3, pluggy-1.3.0 rootdir: /nfs/users/alessandro.candido/qibolab configfile: pyproject.toml testpaths: tests/ plugins: mock-3.12.0, env-1.1.3, dash-2.14.2, cov-4.1.0 collected 2294 items / 2239 deselected / 55 selected tests/test_backends.py xxx [ 5%] tests/test_instruments_erasynth.py sss [ 10%] tests/test_instruments_qblox_cluster_qcm_bb.py .... [ 18%] tests/test_instruments_qblox_cluster_qcm_rf.py .... [ 25%] tests/test_instruments_qblox_cluster_qrm_rf.py .... [ 32%] tests/test_instruments_qutech.py sss [ 38%] tests/test_instruments_rfsoc.py sssss [ 47%] tests/test_instruments_rohde_schwarz.py .... [ 54%] tests/test_instruments_zhinst.py sss [ 60%] tests/test_platform.py ...s.......sss [ 85%] tests/test_result_shapes.py ........ [100%] [...] = 34 passed, 18 skipped, 2239 deselected, 3 xfailed, 14 warnings in 115.71s (0:01:55) = ```

[alecandido]

Tested on Zurich, broken ❌

Pytest report

``` ============================= test session starts ============================== platform linux -- Python 3.10.12, pytest-7.4.3, pluggy-1.3.0 rootdir: /nfs/users/alessandro.candido/qibolab configfile: pyproject.toml testpaths: tests/ plugins: mock-3.12.0, env-1.1.3, dash-2.14.2, cov-4.1.0 collected 2294 items / 2239 deselected / 55 selected tests/test_backends.py xxx [ 5%] tests/test_instruments_erasynth.py sss [ 10%] tests/test_instruments_qblox_cluster_qcm_bb.py ssss [ 18%] tests/test_instruments_qblox_cluster_qcm_rf.py ssss [ 25%] tests/test_instruments_qblox_cluster_qrm_rf.py ssss [ 32%] tests/test_instruments_qutech.py sss [ 38%] tests/test_instruments_rfsoc.py sssss [ 47%] tests/test_instruments_rohde_schwarz.py .... [ 54%] tests/test_instruments_zhinst.py FF. [ 60%] tests/test_platform.py ...........sss [ 85%] tests/test_result_shapes.py ........ [100%] =================================== FAILURES =================================== ____________________ test_experiment_execute_pulse_sequence ____________________ connected_platform = Platform(name='iqm5q', qubits={0: Qubit(name=0, bare_resonator_frequency=5225320060, readout_frequency=5227920060, dri...ed=True, two_qubit_native_types=, topology=) instrument = @pytest.mark.qpu def test_experiment_execute_pulse_sequence(connected_platform, instrument): platform = connected_platform platform.setup() sequence = PulseSequence() > qubits = {0: platform.qubits[0], "c0": platform.qubits["c0"]} E KeyError: 'c0' tests/test_instruments_zhinst.py:769: KeyError ---------------------------- Captured stdout setup ----------------------------- Connected to: Rohde&Schwarz SGS100A (serial:1416.0505k02/113302, firmware:4.2.76.0-4.30.046.295) in 0.20s ------------------------------ Captured log setup ------------------------------ WARNING laboneq.controller.devices.device_zi:device_zi.py:246 HDAWG:dev8660: Include the device options 'HDAWG8/' in the device setup ('options' field of the 'instruments' list in the device setup descriptor). This will become a strict requirement in the future. WARNING laboneq.controller.devices.device_zi:device_zi.py:246 HDAWG:dev8673: Include the device options 'HDAWG4/' in the device setup ('options' field of the 'instruments' list in the device setup descriptor). This will become a strict requirement in the future. WARNING laboneq.controller.devices.device_zi:device_zi.py:246 SHFQC/QA:dev12146: Include the device options 'SHFQC/QC6CH' in the device setup ('options' field of the 'instruments' list in the device setup descriptor). This will become a strict requirement in the future. ______________________ test_experiment_sweep_2d_specific _______________________ connected_platform = Platform(name='iqm5q', qubits={0: Qubit(name=0, bare_resonator_frequency=5225320060, readout_frequency=5227920060, dri...ed=True, two_qubit_native_types=, topology=) instrument = @pytest.mark.qpu def test_experiment_sweep_2d_specific(connected_platform, instrument): platform = connected_platform platform.setup() sequence = PulseSequence() qubits = {0: platform.qubits[0]} swept_points = 5 sequence = PulseSequence() ro_pulses = {} qd_pulses = {} for qubit in qubits: qd_pulses[qubit] = platform.create_RX_pulse(qubit, start=0) sequence.add(qd_pulses[qubit]) ro_pulses[qubit] = platform.create_qubit_readout_pulse( qubit, start=qd_pulses[qubit].finish ) sequence.add(ro_pulses[qubit]) parameter1 = Parameter.relative_phase parameter2 = Parameter.frequency parameter_range_1 = ( np.random.rand(swept_points) if parameter1 is Parameter.amplitude else np.random.randint(swept_points, size=swept_points) ) parameter_range_2 = ( np.random.rand(swept_points) if parameter2 is Parameter.amplitude else np.random.randint(swept_points, size=swept_points) ) sweepers = [] sweepers.append(Sweeper(parameter1, parameter_range_1, pulses=[qd_pulses[qubit]])) sweepers.append(Sweeper(parameter2, parameter_range_2, pulses=[qd_pulses[qubit]])) options = ExecutionParameters( relaxation_time=300e-6, acquisition_type=AcquisitionType.INTEGRATION, averaging_mode=AveragingMode.CYCLIC, ) results = platform.sweep( sequence, options, sweepers[0], sweepers[1], ) > assert len(results[ro_pulses[qubit].serial]) > 0 E TypeError: object of type 'AveragedIntegratedResults' has no len() tests/test_instruments_zhinst.py:856: TypeError [...] FAILED tests/test_instruments_zhinst.py::test_experiment_execute_pulse_sequence FAILED tests/test_instruments_zhinst.py::test_experiment_sweep_2d_specific - ... = 2 failed, 24 passed, 26 skipped, 2239 deselected, 3 xfailed, 14 warnings in 172.07s (0:02:52) = ```

But it's not my fault, it's broken exactly in the same way on main.

[alecandido]

Unfortunately, https://github.com/qiboteam/qibolab_platforms_qrc/pull/88 is outdated, so I don't have a runcard to test tii1q_b1, and thus I can not test whether I'm breaking the RFSoC or not...

[alecandido]

Btw, I obviously can't test on QM, unless someone can point me out a suitable platform to do it...

With this, I'm ready for the review :)

[stavros11]

This I'm trying to understand myself. But, as you can see from the quoted line, Qblox is actually setting this information from the LO at some point, a point when the pulse is accessible (thus during an invocation of some kind of play). So, it is only used to save the information in the meanwhile. We just need to postpone the computation, and propagate the LO instead of the IF.

It is certainly possible to do this and drop _if from the Pulse, but looking at the qblox thing, I think some small refactoring in the driver is needed to make it work. So I would even merge this PR and open an issue to address this from the qblox side. Alternatively you could even drop it from the pulse now and let qblox "inject" it as a new attribute when needed. I believe Python allows it, it's not the best practice, but at least we make sure that is only qblox that is using it.

[alecandido]

Unless you have a different plan for them, such as dropping them at some point (since they're kind of duplicating the pulse.type attribute).

Indeed, the plan is actually to drop. I forgot to spell it out explicitly in #683, where I only detailed the very few additions to main Pulse classes (that is the rationale to drop them as duplicated of the .type attribute, but this remained implicit).

Create a pulse module (directory) and seperate pulse, shape and sequence to different files,

Agreed. I'm trimming the file, but even trimmed it will keep doing quite a bunch of things, so it is useful to split as well.

Move plot methods outside the objects (potentially in another file under the pulse module).

Indeed. I was thinking about a plot module top-level, but plotting is something specific to pulses and their sequences, so it's even nicer as a module in a pulse subpackage.

[alecandido]

Thanks @AleCandido. In terms of hardware deployment, I also played with this branch a bit and it seems to work so it should be safe to merge. Regarding the Zurich tests, I wouldn't worry on this PR, but we should certainly fix them in a different one.

For QM, I would also say it's not worth spending time testing now as I will be doing some major updates in the driver soon to support the Octaves. Also the qua version in our pyproject is very old - they finally put back support for py311 so I will update it when I do the driver. If you really want to try I can provide a runcard (dummy since it is not connected to anything, but still using the real instrument), but not really required.

About hw deployment I wanted to be sure that I was not breaking currently working and used drivers. About the other ones, it's difficult to promise anything, and I hope that after the simplifications it will be simpler to update (in case I'll break anything).

Other than that, I understand that this PR is only a first step and there are several TODOs remaining so it should be fine to even merge as it is.

Indeed, I already opened simplify-pulse-2 locally. But I wanted to avoid a single huge PR, even because they are all potentially breaking, and I wanted to deal with the issues one by one.

[alecandido]

It is certainly possible to do this and drop _if from the Pulse, but looking at the qblox thing, I think some small refactoring in the driver is needed to make it work. So I would even merge this PR and open an issue to address this from the qblox side.

I'm aware of this, and since the Qblox refactoring is actively ongoing (thanks @PiergiorgioButtarini) I believe I can wait for the simplifications happening even there. It's not deadly urgent.

Alternatively you could even drop it from the pulse now and let qblox "inject" it as a new attribute when needed. I believe Python allows it, it's not the best practice, but at least we make sure that is only qblox that is using it.

It is perfectly possible in Python to inject. And, everything possible, it's certainly an option. However, not all possible things are necessarily good ideas (i.e. they are not, in general), and I'd agree with you that it's not a best practice.

For Qblox, to be fair, I don't like either options. So, even the injection is as bad as the current one. But in order to minimize the effort, I will postpone the drop to later. We are aware of the issue, and as soon as @PiergiorgioButtarini or me will fix the usage in Qblox, we could contextually drop the _if attribute.

[PiergiorgioButtarini]

@stavros11 @andrea-pasquale at this point I would actually also like to drop ._if: it is only used by Qblox

❯ rg '\._if'
tests/test_pulses.py
694:        2 * np.pi * pulse._if * pulse.start / 1e9
697:        i, q, num_samples, pulse._if, global_phase + pulse.relative_phase, sampling_rate
719:        == f"Modulated_Waveform_I(num_samples = {num_samples}, amplitude = {format(pulse.amplitude, '.6f').rstrip('0').rstrip('.')}, shape = {str(pulse.shape)}, frequency = {format(pulse._if, '_')}, phase = {format(global_phase + pulse.relative_phase, '.6f').rstrip('0').rstrip('.')})"
723:        == f"Modulated_Waveform_Q(num_samples = {num_samples}, amplitude = {format(pulse.amplitude, '.6f').rstrip('0').rstrip('.')}, shape = {str(pulse.shape)}, frequency = {format(pulse._if, '_')}, phase = {format(global_phase + pulse.relative_phase, '.6f').rstrip('0').rstrip('.')})"
764:        i, q, num_samples, pulse._if, global_phase + pulse.relative_phase, sampling_rate
786:        == f"Modulated_Waveform_I(num_samples = {num_samples}, amplitude = {format(pulse.amplitude, '.6f').rstrip('0').rstrip('.')}, shape = {str(pulse.shape)}, frequency = {format(pulse._if, '_')}, phase = {format(global_phase + pulse.relative_phase, '.6f').rstrip('0').rstrip('.')})"
790:        == f"Modulated_Waveform_Q(num_samples = {num_samples}, amplitude = {format(pulse.amplitude, '.6f').rstrip('0').rstrip('.')}, shape = {str(pulse.shape)}, frequency = {format(pulse._if, '_')}, phase = {format(global_phase + pulse.relative_phase, '.6f').rstrip('0').rstrip('.')})"
834:        2 * np.pi * pulse._if * pulse.start / 1e9
837:        i, q, num_samples, pulse._if, global_phase + pulse.relative_phase, sampling_rate
859:        == f"Modulated_Waveform_I(num_samples = {num_samples}, amplitude = {format(pulse.amplitude, '.6f').rstrip('0').rstrip('.')}, shape = {str(pulse.shape)}, frequency = {format(pulse._if, '_')}, phase = {format(global_phase + pulse.relative_phase, '.6f').rstrip('0').rstrip('.')})"
863:        == f"Modulated_Waveform_Q(num_samples = {num_samples}, amplitude = {format(pulse.amplitude, '.6f').rstrip('0').rstrip('.')}, shape = {str(pulse.shape)}, frequency = {format(pulse._if, '_')}, phase = {format(global_phase + pulse.relative_phase, '.6f').rstrip('0').rstrip('.')})"

src/qibolab/instruments/qblox/controller.py
183:                    pulse._if = int(pulse.frequency - pulse_channel.lo_frequency)

src/qibolab/pulses.py
168:        if abs(pulse._if) * 2 > sampling_rate:
176:            2 * np.pi * pulse._if * time + global_phase + pulse.relative_phase
179:            2 * np.pi * pulse._if * time + global_phase + pulse.relative_phase
200:        modulated_waveform_i.serial = f"Modulated_Waveform_I(num_samples = {num_samples}, amplitude = {format(pulse.amplitude, '.6f').rstrip('0').rstrip('.')}, shape = {str(pulse.shape)}, frequency = {format(pulse._if, '_')}, phase = {format(global_phase + pulse.relative_phase, '.6f').rstrip('0').rstrip('.')})"
202:        modulated_waveform_q.serial = f"Modulated_Waveform_Q(num_samples = {num_samples}, amplitude = {format(pulse.amplitude, '.6f').rstrip('0').rstrip('.')}, shape = {str(pulse.shape)}, frequency = {format(pulse._if, '_')}, phase = {format(global_phase + pulse.relative_phase, '.6f').rstrip('0').rstrip('.')})"

@PiergiorgioButtarini is it possible to store this information anywhere else in Qblox?

@AleCandido I can confirm that pulse._if can be dropped since is only defined in qblox but then never used. I will open a PR to drop this in Qblox.