inconsistent or broken approximate comparisons with `PhasedXPowGate`

[richrines1]

Description of the issue

when its phase_exponent is 0 or 0.5, PhasedXPowGate masquerades as an XPowGate or YPowGate for the value_equality protocol. This can lead to various inconsistent/unexpected/broken behaviors for comparisons involving these gates, including an AssertionError when used in a cirq.Gateset

How to reproduce the issue

gate1 = cirq.PhasedXPowGate(phase_exponent=0)
gate2 = cirq.PhasedXPowGate(phase_exponent=1e-12)
gate3 = cirq.PhasedXPowGate(phase_exponent=2e-12)

assert gate1 == cirq.X  # ok
assert cirq.approx_eq(gate1, cirq.X)  # fails, even though direct equality worked
assert cirq.approx_eq(gate2, gate3)  # ok
assert cirq.approx_eq(gate2, gate1)  # fails, even though they are as close as the previous two

assert cirq.equal_up_to_global_phase(gate1, cirq.X)  # fails, even though direct equality worked
assert cirq.equal_up_to_global_phase(gate2, gate3)  # ok
assert cirq.equal_up_to_global_phase(gate2, gate1)  # fails, even though they are as close as the previous two

_ = cirq.X in cirq.Gateset(gate1)  # AssertionError: X instance matches ... but is not accepted by it.

a few of these problems would be fixed just by defining PhasedXPowGate._value_equality_approximate_values_, but i'm not sure this is enough to make so e.g. both approx_eq(gate1, cirq.X) and approx_eq(gate1, gate2) succeed above. It kinda seems like the ideal fix would be to instead have both XPowGate and YPowGate masquerade as PhasedXPowGates for the sake of comparisons (though maybe this is too severe a change?)

Cirq version

1.4.0.dev20240209232305

[pavoljuhas]

cirq csynkque meeting - the false cirq.approx_eq(gate2, gate1) is a bug that needs to be fixed.

cirq.approx_eq doc gives no guarantees about comparison of different types. I think we should add a trip wire if this == other: return True to the approx_eq evaluation of the cirq.PhasedXPowGate, but in general the result of cirq.approx_eq(gate1, cirq.X) should be False because of different types.

[richrines1]

I think we should add a trip wire if this == other: return True

this would be a big improvement imo (and i think would also fix the Gateset AssertionError above)