Skip to content

Conversation

ddddddanni
Copy link
Contributor

@ddddddanni ddddddanni commented Jun 11, 2025

PR #7236 adds the feature to measure groups of qubit-wise-commuting Pauli operators simultaneously.

However, the current implementation generates a separate readout circuit for each individual Pauli operator within a group. This leads to non-simultaneous measurements, which is corrected in this PR.

@github-actions github-actions bot added the size: M 50< lines changed <250 label Jun 11, 2025
@github-actions github-actions bot added size: S 10< lines changed <50 and removed size: M 50< lines changed <250 labels Jun 11, 2025
@github-actions github-actions bot added size: M 50< lines changed <250 and removed size: S 10< lines changed <50 labels Jun 11, 2025
Copy link

codecov bot commented Jun 11, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.69%. Comparing base (bcd9528) to head (a3d705e).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7416      +/-   ##
==========================================
- Coverage   98.69%   98.69%   -0.01%     
==========================================
  Files        1112     1112              
  Lines       97966    97993      +27     
==========================================
+ Hits        96688    96714      +26     
- Misses       1278     1279       +1     

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ddddddanni ddddddanni marked this pull request as ready for review June 11, 2025 17:02
@ddddddanni ddddddanni requested review from vtomole and a team as code owners June 11, 2025 17:02
Copy link
Collaborator

@eliottrosenberg eliottrosenberg left a comment

Choose a reason for hiding this comment

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

Works great! Thanks, @ddddddanni! Tested in this colab.

@ddddddanni
Copy link
Contributor Author

Works great! Thanks, @ddddddanni! Tested in this colab.

Thank you!! I will wait for Nour to give a stamp and submit this

Copy link
Collaborator

@NoureldinYosri NoureldinYosri left a comment

Choose a reason for hiding this comment

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

thanks @ddddddanni , overall looks good, just a couple of nits + coverage

qubits_to_error: SingleQubitReadoutCalibrationResult, readout_qubits: list[ops.Qid]
) -> SingleQubitReadoutCalibrationResult:
"""Builds a calibration result for the specific readout qubits."""
return SingleQubitReadoutCalibrationResult(
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: this should be a SingleQubitReadoutCalibrationResult method

class SingleQubitReadoutCalibrationResult:
    ...


    def readout_result_for_qubits(self, qubits) -> 'SingleQubitReadoutCalibrationResult':
         ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! Moved the method to under SingleQubitReadoutCalibrationResult

@@ -217,6 +217,31 @@ def _normalize_input_paulis(
return cast(dict[circuits.FrozenCircuit, list[list[ops.PauliString]]], circuits_to_pauli)


def _extract_readout_qubits(pauli_strings: list[ops.PauliString]) -> list[ops.Qid]:
"""Extracts unique qubits from a list of QWC Pauli strings."""
qubits = set()
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit:

Suggested change
qubits = set()
return sorted(set(q for ps in pauli_strings for q in op.qubits))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@ddddddanni ddddddanni requested a review from mrwojtek as a code owner June 23, 2025 22:23
@eliottrosenberg eliottrosenberg added this pull request to the merge queue Jun 23, 2025
Merged via the queue into quantumlib:main with commit b1ff3ae Jun 24, 2025
35 checks passed
BichengYing pushed a commit to BichengYing/Cirq that referenced this pull request Jun 25, 2025
…uli_strings``` method (quantumlib#7416)

[PR quantumlib#7236](quantumlib#7236) adds the
feature to measure groups of qubit-wise-commuting Pauli operators
simultaneously.

However, the current implementation generates a separate readout circuit
for each individual Pauli operator within a group. This leads to
non-simultaneous measurements, which is corrected in this PR.
ddddddanni added a commit to ddddddanni/Cirq that referenced this pull request Jul 15, 2025
…uli_strings``` method (quantumlib#7416)

[PR quantumlib#7236](quantumlib#7236) adds the
feature to measure groups of qubit-wise-commuting Pauli operators
simultaneously.

However, the current implementation generates a separate readout circuit
for each individual Pauli operator within a group. This leads to
non-simultaneous measurements, which is corrected in this PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: M 50< lines changed <250
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants