Skip to content

Group singleshot emulator result extraction - #1509

Merged
alecandido merged 3 commits into
qiboteam:mainfrom
DennisWayo:fix/emulator-group-singleshot-results
Jul 3, 2026
Merged

Group singleshot emulator result extraction#1509
alecandido merged 3 commits into
qiboteam:mainfrom
DennisWayo:fix/emulator-group-singleshot-results

Conversation

@DennisWayo

@DennisWayo DennisWayo commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Summary

This is a follow-up to qiboteam/qibolab #1505 and addresses the singleshot result-handling cleanup described in qiboteam/qibolab #1223.

The change extracts measured subsystem states by grouping acquisitions that share the same sampled full-system state. For each unique measurement sample, the full-system state is unraveled once, then each acquisition reads only the subsystem index it needs.

This replaces the previous diagonal-style extraction through a larger temporary array:

np.stack(np.unravel_index(sampled[inverse_map], dims))[qubit_indices]

with a more direct helper:

_sampled_measurements(sampled, dims, inverse_map, qubit_indices)

The public result shape and clipping behavior are unchanged.

Context

#1505 simplified the cyclic/non-shot probability path. This PR keeps the next result-handling step narrow by touching only the singleshot extraction path.

I kept this independent from #1413, because #1413 still depends on the confusion-matrix path from draft qiboteam/qibolab #1401.

Follow-Up

If this PR lands, my next planned step is to use the merged Qibocal emulator platforms from qiboteam/qibocal #1551 to run Qibocal workflows with the Dynamiqs emulator engine and report concrete validation findings. That follow-up does not depend on #1401 or #1413 (unless @alecandido wants us to take a second look); those remain separate result/confusion-matrix work.

Validation

  • pytest tests/instruments/emulator/test_results.py -q
  • pytest tests/instruments/emulator/test_emulator.py tests/instruments/emulator/test_results.py -q
  • pytest tests/instruments/emulator -q
  • ruff check src/qibolab/_core/instruments/emulator/results.py tests/instruments/emulator/test_results.py
  • ruff format --check src/qibolab/_core/instruments/emulator/results.py tests/instruments/emulator/test_results.py
  • Qibocal Rabi smoke on merged Qibocal platform: qubit acquisition + fit passed
  • Qibocal Rabi smoke on merged Qibocal platform: qutrit acquisition + fit passed

@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 39.47%. Comparing base (326d9a1) to head (960c348).
⚠️ Report is 19 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1509      +/-   ##
==========================================
+ Coverage   39.41%   39.47%   +0.05%     
==========================================
  Files         115      115              
  Lines        6152     6158       +6     
==========================================
+ Hits         2425     2431       +6     
  Misses       3727     3727              
Flag Coverage Δ
unittests 39.47% <100.00%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the emulator single-shot result extraction path to group acquisitions that share the same sampled full-system state, reducing repeated unraveling work while preserving the public result shape and clipping behavior.

Changes:

  • Introduces _sampled_measurements to extract measured subsystem states by grouping acquisitions by shared sampled state.
  • Updates _singleshot_results to use _sampled_measurements instead of the previous diagonal-style extraction through a larger temporary array.
  • Adds a focused unit test validating the acquisition grouping/extraction behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/qibolab/_core/instruments/emulator/results.py Adds _sampled_measurements and switches single-shot extraction to use it.
tests/instruments/emulator/test_results.py Adds a unit test for grouped single-shot measurement extraction.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/qibolab/_core/instruments/emulator/results.py Outdated
@DennisWayo

Copy link
Copy Markdown
Contributor Author

Thanks, this is a fair point. I applied the preallocation change manually rather than accepting the suggestion directly, so I could keep the patch aligned with the existing helper and rerun the local checks before pushing.

_sampled_measurements() now preallocates the output array and fills it per sample, avoiding the intermediate dictionary/stack pass. I also added a small guard for mismatched inverse_map/indices lengths and a focused test for that case.

cc @alecandido

@lballerio lballerio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

good for me

Comment thread src/qibolab/_core/instruments/emulator/results.py Outdated
@lballerio
lballerio marked this pull request as ready for review July 3, 2026 14:22
@lballerio
lballerio requested review from a team July 3, 2026 14:22
@DennisWayo

DennisWayo commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Agreed @lballerio, thanks for catching this. I removed the guard and its dedicated test since inverse_map and indices are both built from the acquisition order in _singleshot_results(). The helper still keeps the preallocated result array and fills it per sampled state.

@lballerio lballerio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the comment before was not critical, now the uninfluent check has been removed, for me is OK

@alecandido
alecandido force-pushed the fix/emulator-group-singleshot-results branch from 961a69d to f17f05c Compare July 3, 2026 15:07
@alecandido
alecandido added this pull request to the merge queue Jul 3, 2026
Merged via the queue into qiboteam:main with commit 56dbdca Jul 3, 2026
1 check was pending
@lballerio lballerio mentioned this pull request Jul 9, 2026
12 tasks
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.

5 participants