Add pauli_rep diagonalization #6113
Merged
Conversation
This file contains 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
|
Hello. You may have forgotten to update the changelog!
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6113 +/- ##
==========================================
+ Coverage 98.72% 99.60% +0.88%
==========================================
Files 446 446
Lines 42934 42492 -442
==========================================
- Hits 42386 42326 -60
+ Misses 548 166 -382 ☔ View full report in Codecov by Sentry. |
…ane into pauli_rep_diagonalize
PietropaoloFrisoni
approved these changes
Sep 4, 2024
There was a problem hiding this comment.
Thanks, this was quite instructive to study.
I think in the PR branch there are some typos in doc (for example supported_base_obs), but it looks like these have been fixed so it would only be necessary to merge from master.
I don't want to block this PR since it seems good to me. Still, I think I should not be the final reviewer since I still lack sufficient context about this project. Potentially, someone who reviewed the first PR and who is more familiar with Pauli words would be more recommended for the final approval.
mudit2812
reviewed
Sep 6, 2024
mudit2812
reviewed
Sep 6, 2024
mudit2812
reviewed
Sep 6, 2024
mudit2812
reviewed
Sep 6, 2024
mudit2812
approved these changes
Sep 6, 2024
lillian542
commented
Sep 6, 2024
Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>
mudit2812
added a commit
that referenced
this pull request
Sep 10, 2024
**Context:** The diagonalization transform should default to the faster pauli implementation if all the observables have a `pauli_rep` and we are diagonalizing everything to the measurement basis. **Description of the Change:** Check if above contitions apply, and if so, use diagonalization via `rotations_and_diagonal_measurements` and `diagonalize_qwc_pauli_words` instead. Allow conversion to either `Z` or `eigvals` for both methods. Also includes a bug fix where the fallback method was failing to raise an error for non-commuting measurements when one of the measurements had no observable (like `qml.sample()`). **Benefits:** A common use case (all pauli words, everything has to be in the measurement basis) is diagonalized more efficiently. **Related Shortcut Stories:** [sc-60179] --------- Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>
mudit2812
added a commit
that referenced
this pull request
Sep 12, 2024
**Context:** The diagonalization transform should default to the faster pauli implementation if all the observables have a `pauli_rep` and we are diagonalizing everything to the measurement basis. **Description of the Change:** Check if above contitions apply, and if so, use diagonalization via `rotations_and_diagonal_measurements` and `diagonalize_qwc_pauli_words` instead. Allow conversion to either `Z` or `eigvals` for both methods. Also includes a bug fix where the fallback method was failing to raise an error for non-commuting measurements when one of the measurements had no observable (like `qml.sample()`). **Benefits:** A common use case (all pauli words, everything has to be in the measurement basis) is diagonalized more efficiently. **Related Shortcut Stories:** [sc-60179] --------- Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Context:
The diagonalization transform should default to the faster pauli implementation if all the observables have a
pauli_repand we are diagonalizing everything to the measurement basis.Description of the Change:
Check if above contitions apply, and if so, use diagonalization via
rotations_and_diagonal_measurementsanddiagonalize_qwc_pauli_wordsinstead. Allow conversion to eitherZoreigvalsfor both methods.Also includes a bug fix where the fallback method was failing to raise an error for non-commuting measurements when one of the measurements had no observable (like
qml.sample()).Benefits:
A common use case (all pauli words, everything has to be in the measurement basis) is diagonalized more efficiently.
Related Shortcut Stories:
[sc-60179]