Skip to content

deduplicate waveforms in qblox#1381

Open
RoyStegeman wants to merge 8 commits intomainfrom
deduplicate_qblox
Open

deduplicate waveforms in qblox#1381
RoyStegeman wants to merge 8 commits intomainfrom
deduplicate_qblox

Conversation

@RoyStegeman
Copy link
Member

@RoyStegeman RoyStegeman commented Jan 29, 2026

closes #1376

@codecov
Copy link

codecov bot commented Jan 29, 2026

Codecov Report

❌ Patch coverage is 0% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.03%. Comparing base (e618b6d) to head (e88b177).

Files with missing lines Patch % Lines
...olab/_core/instruments/qblox/sequence/waveforms.py 0.00% 19 Missing ⚠️
...bolab/_core/instruments/qblox/sequence/sequence.py 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1381      +/-   ##
==========================================
- Coverage   39.10%   39.03%   -0.07%     
==========================================
  Files         114      114              
  Lines        5674     5684      +10     
==========================================
  Hits         2219     2219              
- Misses       3455     3465      +10     
Flag Coverage Δ
unittests 39.03% <0.00%> (-0.07%) ⬇️

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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@alecandido alecandido left a comment

Choose a reason for hiding this comment

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

Just a comment on the type hint.

As mentioned, the np.unique function can be used to simplify the implementation.

We could also add another small improvement, just in terms of performances, deduplicating at the level of Pulse objects, rather than evaluated waveforms. This will just save the repeated evaluation

amplitude_swept: set[PulseId],
duration_swept: dict[PulseLike, Sweeper],
) -> dict[ComponentId, WaveformSpec]:
) -> Union[dict[WaveformInd, WaveformSpec], WaveformIndices]:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
) -> Union[dict[WaveformInd, WaveformSpec], WaveformIndices]:
) -> tuple[dict[WaveformInd, WaveformSpec], WaveformIndices]:

Copy link
Member

@alecandido alecandido left a comment

Choose a reason for hiding this comment

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

The current review is meant as "adiabatic". But I believe we can even do it without the cache and np.unique() on the waveforms. Let me investigate


class Q1Sequence(Model):
waveforms: Waveforms
waveforms: dict[WaveformInd, Waveform]
Copy link
Member

Choose a reason for hiding this comment

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


ComponentId = tuple[UUID4, int]
WaveformIndices = dict[ComponentId, tuple[int, int]]
WaveformInd = int
Copy link
Member

Choose a reason for hiding this comment

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

We can afford two more characters (on top of the eleven...)

Suggested change
WaveformInd = int
WaveformIndex = int

duration=int(duration_),
)

# we do this since pulse is not hashable
Copy link
Member

Choose a reason for hiding this comment

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

In [1]: from qibolab import Pulse, Rectangular, Readout, Acquisition
In [2]: p = Pulse(amplitude=0.3, duration=40, envelope=Rectangular())
In [3]: r = Readout(acquisition=Acquisition(duration=30), probe=p)
In [4]: {p: 3, r: 4}
Out[4]:
{Pulse(id_=UUID('e5069a81-3a6e-4fee-a0f6-eaab0da119e2'), kind='pulse', duration=40.0, amplitude=0.3, envelope=Rectangular(kind='rectangular'), relative_phase=0.0): 3,
 Readout(id_=UUID('768f4f08-ca1e-4cc4-8839-cd48b93cccaf'), kind='readout', acquisition=Acquisition(id_=UUID('e6f84555-878a-4f31-b718-83839065378e'), kind='acquisition', duration=30.0), probe=Pulse(id_=UUID('e5069a81-3a6e-4fee-a0f6-eaab0da119e2'), kind='pulse', duration=40.0, amplitude=0.3, envelope=Rectangular(kind='rectangular'), relative_phase=0.0)): 4}
In [6]: hash(p)
Out[6]: -853604456920802955
In [7]: hash(r)
Out[7]: 7280396492324559753

?

# we do this since pulse is not hashable
cache = {}

def waveform_cached(pulse, component, duration=None):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def waveform_cached(pulse, component, duration=None):
def waveform_cached(pulse: Pulse, component: str, duration: Optional[float] = None) -> WaveformSpec:

Comment on lines +109 to +111
waveform_specs = {
idx: waveforms[np.where(inverse_idx == idx)[0][0]] for idx in unique_idx
}
Copy link
Member

Choose a reason for hiding this comment

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

The inverse_idx is indexing the unique values, while idx in unique_idx is indexing the original array. This np.where() seems actually wrong

In [30]: a
Out[30]: array(['c', 'a', 'a', 'a', 'c', 'a', 'a', 'c', 'c', 'b'], dtype='<U1')
In [31]: _, uni, inv = np.unique(a, return_inverse=True, return_index=True)
In [32]: np.where(inv == uni[1])
Out[32]: (array([], dtype=int64),)

Copy link
Member

@alecandido alecandido Jan 30, 2026

Choose a reason for hiding this comment

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

The correct one should be directly

Suggested change
waveform_specs = {
idx: waveforms[np.where(inverse_idx == idx)[0][0]] for idx in unique_idx
}
waveform_specs = {idx: waveforms[idx] for idx in unique_idx}

since unique_idx is meant to be the index where to unique values in bytes_ are taken from, and they should correspond to the same index in waveforms.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, maybe even

Suggested change
waveform_specs = {
idx: waveforms[np.where(inverse_idx == idx)[0][0]] for idx in unique_idx
}
waveform_specs = {inverse_idx[idx]: waveforms[idx] for idx in unique_idx}

since the indices included in waveform_indices are actually the inverse ones

@alecandido
Copy link
Member

Ok, working with Pulses is actually convoluted, since we did not add an extensive high-level interface. But it is actually possible to do what I had in mind:

In [42]: ps
Out[42]:
[Pulse(id_=UUID('e5069a81-3a6e-4fee-a0f6-eaab0da119e2'), kind='pulse', duration=40.0, amplitude=0.3, envelope=Rectangular(kind='rectangular'), relative_phase=0.0),
 Pulse(id_=UUID('aaca1878-3b5a-4f7a-bb47-f8070476f1f2'), kind='pulse', duration=40.0, amplitude=0.3, envelope=Gaussian(kind='gaussian', rel_sigma=0.1), relative_phase=0.0),
 Pulse(id_=UUID('c700876d-8bdf-4de5-a786-1dabb5778a16'), kind='pulse', duration=40.0, amplitude=1e-07, envelope=Rectangular(kind='rectangular'), relative_phase=0.0)]

In [43]: ps_ = np.random.choice(ps, 10)

In [44]: ps_
Out[44]:
array([Pulse(id_=UUID('c700876d-8bdf-4de5-a786-1dabb5778a16'), kind='pulse', duration=40.0, amplitude=1e-07, envelope=Rectangular(kind='rectangular'), relative_phase=0.0),
       Pulse(id_=UUID('aaca1878-3b5a-4f7a-bb47-f8070476f1f2'), kind='pulse', duration=40.0, amplitude=0.3, envelope=Gaussian(kind='gaussian', rel_sigma=0.1), relative_phase=0.0),
       Pulse(id_=UUID('c700876d-8bdf-4de5-a786-1dabb5778a16'), kind='pulse', duration=40.0, amplitude=1e-07, envelope=Rectangular(kind='rectangular'), relative_phase=0.0),
       Pulse(id_=UUID('e5069a81-3a6e-4fee-a0f6-eaab0da119e2'), kind='pulse', duration=40.0, amplitude=0.3, envelope=Rectangular(kind='rectangular'), relative_phase=0.0),
       Pulse(id_=UUID('c700876d-8bdf-4de5-a786-1dabb5778a16'), kind='pulse', duration=40.0, amplitude=1e-07, envelope=Rectangular(kind='rectangular'), relative_phase=0.0),
       Pulse(id_=UUID('aaca1878-3b5a-4f7a-bb47-f8070476f1f2'), kind='pulse', duration=40.0, amplitude=0.3, envelope=Gaussian(kind='gaussian', rel_sigma=0.1), relative_phase=0.0),
       Pulse(id_=UUID('aaca1878-3b5a-4f7a-bb47-f8070476f1f2'), kind='pulse', duration=40.0, amplitude=0.3, envelope=Gaussian(kind='gaussian', rel_sigma=0.1), relative_phase=0.0),
       Pulse(id_=UUID('aaca1878-3b5a-4f7a-bb47-f8070476f1f2'), kind='pulse', duration=40.0, amplitude=0.3, envelope=Gaussian(kind='gaussian', rel_sigma=0.1), relative_phase=0.0),
       Pulse(id_=UUID('c700876d-8bdf-4de5-a786-1dabb5778a16'), kind='pulse', duration=40.0, amplitude=1e-07, envelope=Rectangular(kind='rectangular'), relative_phase=0.0),
       Pulse(id_=UUID('aaca1878-3b5a-4f7a-bb47-f8070476f1f2'), kind='pulse', duration=40.0, amplitude=0.3, envelope=Gaussian(kind='gaussian', rel_sigma=0.1), relative_phase=0.0)],
      dtype=object)

In [52]: ps__ = [Pulse.model_validate(p.model_dump()) for p in ps_]

In [53]: ps__
Out[53]:
[Pulse(id_=UUID('fa913bc5-8b30-4bf5-8ec7-7a58b875253e'), kind='pulse', duration=40.0, amplitude=1e-07, envelope=Rectangular(kind='rectangular'), relative_phase=0.0),
 Pulse(id_=UUID('f58f1d30-978c-43cf-ac26-1868a3b04f5b'), kind='pulse', duration=40.0, amplitude=0.3, envelope=Gaussian(kind='gaussian', rel_sigma=0.1), relative_phase=0.0),
 Pulse(id_=UUID('32a3df43-d53b-4a5d-81e4-449f2929f665'), kind='pulse', duration=40.0, amplitude=1e-07, envelope=Rectangular(kind='rectangular'), relative_phase=0.0),
 Pulse(id_=UUID('5a568694-3d10-49bc-9cf6-57078fedcf2a'), kind='pulse', duration=40.0, amplitude=0.3, envelope=Rectangular(kind='rectangular'), relative_phase=0.0),
 Pulse(id_=UUID('e052dd88-5c03-49cd-a683-9ed43151bd46'), kind='pulse', duration=40.0, amplitude=1e-07, envelope=Rectangular(kind='rectangular'), relative_phase=0.0),
 Pulse(id_=UUID('ce15f263-8484-4ce1-85be-f79848dca8b7'), kind='pulse', duration=40.0, amplitude=0.3, envelope=Gaussian(kind='gaussian', rel_sigma=0.1), relative_phase=0.0),
 Pulse(id_=UUID('f2ee585d-6e18-4c55-94f6-c871212cba71'), kind='pulse', duration=40.0, amplitude=0.3, envelope=Gaussian(kind='gaussian', rel_sigma=0.1), relative_phase=0.0),
 Pulse(id_=UUID('19fa6f9f-8f09-4b9d-ad60-f8863ecbdf18'), kind='pulse', duration=40.0, amplitude=0.3, envelope=Gaussian(kind='gaussian', rel_sigma=0.1), relative_phase=0.0),
 Pulse(id_=UUID('b59d159f-4923-4d99-99c8-51d76db9de09'), kind='pulse', duration=40.0, amplitude=1e-07, envelope=Rectangular(kind='rectangular'), relative_phase=0.0),
 Pulse(id_=UUID('8ce16341-4dfa-4429-afde-b078cfe2ea3e'), kind='pulse', duration=40.0, amplitude=0.3, envelope=Gaussian(kind='gaussian', rel_sigma=0.1), relative_phase=0.0)]
In [55]: [hash(p) for p in ps__]
Out[55]:
[2182729606208933173,
 -5036063928699656048,
 2182729606208933173,
 -853604456920802955,
 2182729606208933173,
 -5036063928699656048,
 -5036063928699656048,
 -5036063928699656048,
 2182729606208933173,
 -5036063928699656048]

In [56]: np.unique([hash(p) for p in ps__])
Out[56]: array([-5036063928699656048,  -853604456920802955,  2182729606208933173])

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Waveforms are not deduplicated in qblox resulting in hitting memory limit

2 participants