Allow EigenGate phase shifts to be specified per eigenspace

[daxfohl]

Starts #1731, fixes #7052. Also fixes #6764.

No gates have been instrumented with this yet. This is just a minimally-invasive foundation. It allows EigenGate.global_phase to be passed in as a tuple[float] instead of a single float, with the implication that each value in the tuple will offset the eigenshift of the corresponding eigencomponent by that amount, rather than a single offset that applies to all eigenshifts. The offset is applied pre-exponentiation, just as in the current global shift.

This approach made the equality check faster and simpler, as there is no need to calculate the period or canonical exponent, which also allowed simplification and speedup of _equal_up_to_global_phase_.

It also highlighted a bug in the existing code, seen in the change to common_gates_test. A ZPowGate(shift)**5 should be equal for shifts of 0.1 and 0.5. The old code did not exponentiate the shift in the equality check.

Update [2/11/24]

A more isolated fix for the above bugs is at #7057. After having worked on that in isolation, it occurs to me that the current PR may not be what was intended by #1731. The approach in the current PR is to leave exponent behaving as-is, and allow users to set the phase offset per eigen space rather than as a global offset. This allows users to have more fine-grained control on the gate's unitary effect. However, this approach still has an extra degree of freedom, which also makes it challenging to determine a canonical representation of the gate.

Stepping back, perhaps the intent in #1731 is to eliminate the exponent entirely, and replace it with the the phases themselves, one for each eigenspace. Essentially, the _value_equality_values_. This seems to be the approach taken with PauliStringPhasor, with two exponent terms and nothing else. That would make things more precise and eliminate ambiguity, though at the possible expense of ease-of-use, since exponentiating a gate would mean users have to manually change each phase. (That said, we could provide a helper function that exponentiates all of them. We could even make it only accept integers, so we don't have to choose between f_inv(f(x))==x and x==y implies f(x)==f(y)). That approach would be a significantly bigger change though. Not necessarily bigger in amount of code, but backwards compatibility and user ergonomics would be significantly different.

It's probably worth syncing on the original intent before committing to the approach in this PR. cc @Strilanc @viathor

Update 2 [2/11/24]

I still think this approach makes sense. A distinction between phases and exponent may improve ergonomics in some cases. This approach allows that. If users want to exponentiate a gate, great. If users want to set eigenshifts manually, we can do that too. If they want to do both, that's fine. If they want to do X**3**(1/3) and interpret that as X**(1/3) instead of X (i.e. if they prefer x==y implies f(x)==f(y) over f_inv(f(x))==x, they'll be sad, but so it goes. It's not likely to be important, and there are workarounds.

An open question, if the user provides shifts, should they be interpreted as offsets, or as replacements? Either way works. Offsets fit in better with the current global shift, and would allow us to continue treating global shift as an identical offset to each eigenspace. But if users tend to think in terms of "setting" the eigenshift, then we could interpret them that way (and possibly keep global shift as its own input that adds an extra global shift to those settings, which may be useful as a parameter). Either way works. And I assume exponentiation always happens after the shifts are applied, since the notation X(shift=a)**b would be surprising otherwise.

Update 3 [2/12/24]

After asking the open question above, the idea of having a separate parameter for "setting" the eigenvalues, while retaining a global_shift parameter for shifting them in unison, seemed appealing because it would automatically be backwards compatible, while allowing users to choose between shifting eigenvalues globally and setting eigenvalues individually, which seems to make sense ergonomically. So I started looking into that approach. It didn't seem like it would be a very big change from what I've already done in this PR.

As it turns out, this is already supported. _eigencomponents and _eigenshifts are abstract methods on EigenGate, and gate implementations can override them however they wish. Adding an extra field to EigenGate to override those overrides is superfluous.

Therefore, there's really nothing to do here at the EigenGate level. In gates that wish to allow custom eigenshift values, they simply have to add that parameter to their constructor, and update their _eigencomponents implementation to respect those inputs.

It's probably not how we'd have implemented it if starting from scratch; it's slightly awkward for exponent and phase to be input fields, while eigenshifts is an abstract method. But OTOH, sometimes the eigenshifts are calculated, so, maybe it does make sense for them to have a different way of representing them.

Anyway, I'll be closing this PR, and leaving #7057 open just for the associated bugfix. I'll provide additional context on the original issue #1731 too.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.18%. Comparing base (60f2590) to head (6a5c3af).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7051      +/-   ##
==========================================
- Coverage   98.18%   98.18%   -0.01%     
==========================================
  Files        1089     1089              
  Lines       95202    95222      +20     
==========================================
+ Hits        93473    93490      +17     
- Misses       1729     1732       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.