Results collection

[alecandido]

Closes #809

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 51.63%. Comparing base (9ad9c1a) to head (a7ef4ec).
Report is 15 commits behind head on 0.2.

Additional details and impacted files

@@            Coverage Diff             @@
##              0.2     #964      +/-   ##
==========================================
+ Coverage   51.45%   51.63%   +0.17%     
==========================================
  Files          57       57              
  Lines        2740     2750      +10     
==========================================
+ Hits         1410     1420      +10     
  Misses       1330     1330              

Flag	Coverage Δ
unittests	51.63% <100.00%> (+0.17%)	⬆️

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.

[alecandido]

Most of the work was of course done in #940, but here I'd like to discuss another possible breaking change, i.e.

qibolab/src/qibolab/platform/platform.py

Lines 317 to 321 in adee9c4

	results = defaultdict(list)
	for b in batch(sequences, self._controller.bounds):
	result = self._execute(b, options, configs, integration_setup, sweepers)
	for serial, data in result.items():
	results[serial].append(data)

we are now creating lists for each pulse, but in principle pulses may be unique, and we could just keep the results flatter (replacing the dict[PulseId, list[Result]] with dict[PulseId, Result]).

Of course, this is only relevant for unrolling, since in all the other cases there is a single pulse with the given id, and if there are many results associated (e.g. as in sweepers) the structure is internal to the Results array.

The drawback of the proposed scenario is that we lift to the user the burden of generating unique acquisition pulses (of course, we can validate the sequence to check for duplicates) making copies while generating the sequences (instead of reusing the same pulse - but a .copy() or copy.copy() call is sufficient), while those of the current situation are that you're almost always receiving lists of one element, and that you need to explicitly avoid copies, if you want to keep the results together.

[stavros11]

The drawback of the proposed scenario is that we lift to the user the burden of generating unique acquisition pulses (of course, we can validate the sequence to check for duplicates) making copies while generating the sequences (instead of reusing the same pulse - but a .copy() or copy.copy() call is sufficient), while those of the current situation are that you're almost always receiving lists of one element, and that you need to explicitly avoid copies, if you want to keep the results together.

I would say this solution is acceptable. We just need to validate in platform.execute that every pulse appears at most once in the given list of sequences (not even in different sequences that are given in the same list). I don't see any serious issue with that limitation, but if there is we could even relax it by checking only acquisition pulses (and allow reusing drive or other types), as only acquisition is relevant for results.

[alecandido]

I don't see any serious issue with that limitation, but if there is we could even relax it by checking only acquisition pulses (and allow reusing drive or other types), as only acquisition is relevant for results.

Then, I will rebase, provide the check (and a couple of tests), and then ask for a review.

[alecandido]

@stavros11 now this PR should be feature-complete, I'm just missing suitable tests (and fixing the other ones), and we are done.

[stavros11]

Thanks @alecandido, looks good to me. I also tested the single shot routine with and without unrolling and some quick circuit executions and they all seem to work.

[alecandido]

Since it has already been approved, I'll merge as soon as the CI passes