Add qubits to PauliStringPhasor#5565
Merged
dabacon merged 25 commits intoquantumlib:masterfrom Jun 24, 2022
Merged
Conversation
tanujkhattar
requested changes
Jun 21, 2022
Collaborator
tanujkhattar
left a comment
tanujkhattar
left a comment
There was a problem hiding this comment.
Looks good overall. Can you please also mark this PR as a breaking change because it changes the diagrams, repr and json of PauliStringPhasor?
dabacon
added
BREAKING CHANGE
Co-authored-by: Tanuj Khattar <tanujkhattar@google.com>
Co-authored-by: Tanuj Khattar <tanujkhattar@google.com>
Co-authored-by: Tanuj Khattar <tanujkhattar@google.com>
Co-authored-by: Tanuj Khattar <tanujkhattar@google.com>
Collaborator
Author
|
Thanks @tanujkhattar for the quick review! |
Collaborator
|
Nice, +1 to this approach! |
Collaborator
Author
|
OK, ready for rereview |
tanujkhattar
approved these changes
Jun 23, 2022
| # Use qubits below instead of qubits or pauli_string.qubits | ||
| gate = PauliStringPhasorGate( | ||
| pauli_string.dense(pauli_string.qubits), | ||
| pauli_string.dense(qubits or pauli_string.qubits), |
Collaborator
There was a problem hiding this comment.
Suggested change
| pauli_string.dense(qubits or pauli_string.qubits), | |
| pauli_string.dense(qubits), |
| exponent_pos=exponent_pos, | ||
| ) | ||
| super().__init__(gate, pauli_string.qubits) | ||
| super().__init__(gate, qubits or pauli_string.qubits) |
Collaborator
There was a problem hiding this comment.
Suggested change
| super().__init__(gate, qubits or pauli_string.qubits) | |
| super().__init__(gate, qubits) |
Co-authored-by: Tanuj Khattar <tanujkhattar@google.com>
Co-authored-by: Tanuj Khattar <tanujkhattar@google.com>
pavoljuhas
approved these changes
Jun 24, 2022
Collaborator
pavoljuhas
left a comment
pavoljuhas
left a comment
There was a problem hiding this comment.
LGTM after mypy error is resolved.
dabacon
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
Collaborator
|
Automerge cancelled: A status check is failing. |
CirqBot
removed
automerge
rht
pushed a commit
to rht/Cirq
that referenced
this pull request
May 1, 2023
harry-phasecraft
pushed a commit
to PhaseCraft/Cirq
that referenced
this pull request
Oct 31, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently PauliStringPhasor determines its qubits from the PauliString passed in to it. PauliStringPhasorGate is based on DensePauliString, which allows Pauli Identity gates. When PauliStringPhasorGate is used create a PauliStringPhasor, the DensePauliString is converted to a PauliString, dropping identity gates. This causes #5167, which fails
gate(*qubits).gate == gate.This fixes this problem by including all of the qubits when going from a PauliStringPhasorGate into the qubits on the PauliStringPhasor operation, and including these in the operation's gate.
This also fixes a #5564 where PauliString does preserve qubit order in it's repr.
Fixes #5167
Fixes #5564
This is a breaking change because it updates the reprs of PauliStringPhasor and PauliString. It also changes the json, though it is backwards compatible in this case.