Skip to content

Conversation

@ddddddanni
Copy link
Collaborator

I split this #7492 giant PR into smaller pieces for easier review.
This PR:

  1. Adds a CircuitToPauliStringsParameters class which supports postselection_symmetries.
  2. Modify the validation method
  3. Allows measure_pauli_strings to take CircuitToPauliStringsParameters as a input.

@ddddddanni ddddddanni requested review from a team and vtomole as code owners November 11, 2025 19:50
@github-actions github-actions bot added the size: L 250< lines changed <1000 label Nov 11, 2025
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.

This PR doesn't actually apply the postselection, correct? If so, we should raise a NotImplementedError if the user tries to use postselection symmetries.

@ddddddanni ddddddanni marked this pull request as ready for review November 19, 2025 01:10
@ddddddanni
Copy link
Collaborator Author

This PR doesn't actually apply the postselection, correct? If so, we should raise a NotImplementedError if the user tries to use postselection symmetries.

Done!

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.

One small nit, otherwise LGTM. Thanks!


circuit: circuits.FrozenCircuit
pauli_strings: list[list[ops.PauliString]]
postselection_symmetries: list[tuple[ops.PauliString | ops.PauliSum, int]]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we have this default to an empty list? That way users don't need to enter anything for postselection_symmetries if they don't want to postselect.

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.

LGTM%nits

"""

circuit: circuits.FrozenCircuit
pauli_strings: list[list[ops.PauliString]]
Copy link
Collaborator

Choose a reason for hiding this comment

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

since this is a frozen class, use tuple instead

all_qubits: list[ops.Qid] | frozenset[ops.Qid],
) -> bool:
"""Checks if a Pauli sum and a Pauli string are Qubit-Wise Commuting."""
for pauli_sum_term in pauli_sum:
Copy link
Collaborator

Choose a reason for hiding this comment

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

reuse the _are_two_pauli_strings_qubit_wise_commuting method

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

Labels

size: L 250< lines changed <1000

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants