-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix EigenGate equality #7057
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix EigenGate equality #7057
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7057 +/- ##
==========================================
- Coverage 98.66% 98.65% -0.01%
==========================================
Files 1106 1106
Lines 95914 95852 -62
==========================================
- Hits 94630 94564 -66
- Misses 1284 1288 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beautiful! Thank you!
@Strilanc can you give this change a quick look? I want to make sure I'm not making any incorrect assumptions before cutting 1.5, especially given the logic changed the results of an equality test. |
Adding BREAKING_CHANGE label since it changes an existing unit test. The behavior of that unit test was ostensibly incorrect, as discussed in #7052. Though the implementation is very different from previous implementation, and hard to compare, the sum effect of this change is:
Thus, some gates that were previously incorrectly considered unequal are now equal, as issue #7052 describes. There are no cases where gates that were previously considered equal are now unequal (if the canonical exponent is the same and the unexponentiated global phase is the same, then the exponentiated/periodized global phase will be the same). |
* Fix equality for EigenGates * Handle zero-phase with symbolic exponents * Simplify code * Remove superfluous _value_equality_values_ * more tests * more tests * Fix pauli_interaction_gate change * Remove unnecessary EigenGate._value_equality_approximate_values_ override * resort imports * sort imports * remove unused import * Add details to EigenGate._value_equality_values_ docstring
Fixes #7052. Also fixes #6764.
EigenGates
of the same class generally differ only by the phases that they apply to each eigenspace. Therefore their_value_equality_values_
can be determined uniquely by calculating these phases. This is both faster (no need to calculate the period or canonical exponent) and more robust (exponentiation of phases is accounted for) than the existing implementation. The simplification also allows it to handle large global shifts that cause division-by-zero errors in period calculations (#6764). Additionally, many different edge cases with symbolic exponents are easier to identify and account for.This also allows us to get rid of some superfluous
_value_equality_values_
implementations in subclasses, except where the eigenspaces themselves are changeable.