Hi Dax and Pavol,

Thank you a lot for your feedback! I updated the code following your suggestion.

However, in the meantime I noticed that several tests were failing. I updated the tests so that you can see the effect, but I think this change is overall problematic.
The problem is that:

• circuit optimization,
• merge gate operations,
• and conversion to proto,

all transform gates X and Y to PhasedXPowGate. This worked well until now because the gate was correctly interpreted as X or Y, for example when printing the circuit. However, with this change this is no longer the case. In particular, users will be surprised to optimize a circuit and instead of X, end up with a PhasedXPowGate(phase_exponent=0). It's harder to read, and it's a poor user experience.

I am inclined to drop this pull request. Let me know what you think and if there is a way around this problem.

Should an optimizer have a final stage where it puts all the gates in canonical form perhaps?

Should an optimizer have a final stage where it puts all the gates in canonical form perhaps?

Indeed, that would be nice. This change actually shows the affected transformations could work better.

@tanujkhattar - would you have some tips for a transformer that does such conversion,
e.g., cirq.PhasedXPowGate(phase_exponent=0) to cirq.X?

Here is an example code adapted from a unit test -

import cirq
q = cirq.q(0)
c0 = cirq.Circuit([cirq.Moment([cirq.Z(q) ** 0.5]), cirq.Moment([cirq.Y(q) ** 0.25])])
c1 = cirq.eject_z(c0)
print(repr(c1))

output:

cirq.Circuit([
    cirq.Moment(
        cirq.PhasedXPowGate(phase_exponent=0.0, exponent=0.25).on(cirq.LineQubit(0)),
    ),
    cirq.Moment(
        cirq.S(cirq.LineQubit(0)),
    ),
])

I am happy to try and write a transformer that does what you suggested, Pavol.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.65%. Comparing base (02941bf) to head (3034d1a).
Report is 34 commits behind head on main.

@@            Coverage Diff             @@
##             main    #7070      +/-   ##
==========================================
- Coverage   98.65%   98.65%   -0.01%     
==========================================
  Files        1106     1106              
  Lines       95869    95892      +23     
==========================================
+ Hits        94581    94603      +22     
- Misses       1288     1289       +1     

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

@codrut3 thank you for working on this and your willingness to persist.

It seems like this would be a worthwhile PR to finish (rather than close) if a transformation can be implemented per comments above. The Qualtran team are among the best people to consult on this topic; @pavoljuhas tagged Tanuj in a comment above, but he may not have been available. I'll ask on the Qualtran list – maybe someone else will be able to help.

If we don't get a solution within 24 hrs, I think we should push this PR to after the Cirq 1.5 release. Do people agree?

@codrut3 - I am giving it a quick review pass, please hold on till my next comment.

@codrut3 - thank you for fixing this! (We are a bit in rush with next
release so I took a liberty to wrap up few final steps.)

Thank you a lot for doing this @pavoljuhas! I really appreciate it! I also learned from your commits :)

I think we should consider reverting this. This is backwards incompatible:

cirq.X**0.5 == cirq.PhasedXPowGate(exponent=0.5,phase_exponent=0) returns True before the change and False afterwards. This is a change in behavior that could have unexpected and adverse behavior for users.

cross referencing comment in the related issue - #7284 (comment)

TLDR - let us wait with reverting if we can adjust for this downstream