Skip to content
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

Xa*Zb*Zb*Zb works but Xa*Zb**3 raises error #5715

Closed
tanujkhattar opened this issue Jul 11, 2022 · 9 comments · Fixed by #6618
Closed

Xa*Zb*Zb*Zb works but Xa*Zb**3 raises error #5715

tanujkhattar opened this issue Jul 11, 2022 · 9 comments · Fixed by #6618
Assignees
Labels
area/paulis kind/bug-report Something doesn't seem to work. triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add unitaryhack

Comments

@tanujkhattar
Copy link
Collaborator

How to reproduce the issue

In [1]: import cirq
   ...: a, b = cirq.LineQubit.range(2)
   ...: Xa, Zb = cirq.X(a), cirq.Z(b)
   ...: print(Xa * Zb * Zb * Zb) # Works because always stays a `cirq.PauliString`. 
   ...: print(Xa * Zb ** 3) # Fails because Zb ** 3 returns a `cirq.ZPowGate` instead of `cirq._PauliZ`; which doesn't support multiplication with other pauli's.

X(q(0))*Z(q(1))
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-2-402c68ab82e4> in <module>
      3 Xa, Zb = cirq.X(a), cirq.Z(b)
      4 print(Xa * Zb * Zb * Zb) # Works because always stays a `cirq.PauliString`.
----> 5 print(Xa * Zb ** 3) # Fails because Zb ** 3 returns a `cirq.ZPowGate` instead of `cirq._PauliZ`; which doesn't support multiplication with other pauli's.

TypeError: unsupported operand type(s) for *: 'SingleQubitPauliStringGateOperation' and 'GateOperation'

Cirq version:
0.16.0.dev

@tanujkhattar tanujkhattar added the kind/bug-report Something doesn't seem to work. label Jul 11, 2022
@tanujkhattar tanujkhattar changed the title $X_{a} * Z_{b} * Z_{b} * Z_{b}$ works but $X_{a} * Z_{b}^{3}$ raises error Xa * Zb * Zb * Zb works but Xa * Zb ** 3 raises error Jul 11, 2022
@tanujkhattar tanujkhattar changed the title Xa * Zb * Zb * Zb works but Xa * Zb ** 3 raises error Xa*Zb*Zb*Zb works but Xa*Zb**3 raises error Jul 11, 2022
@tanujkhattar tanujkhattar added the triage/discuss Needs decision / discussion, bring these up during Cirq Cynque label Jul 14, 2022
@tanujkhattar
Copy link
Collaborator Author

From cirq sync:

Looks like a bug and should be fixed. Should not be categorized as a breaking change.

@tanujkhattar tanujkhattar added triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add and removed triage/discuss Needs decision / discussion, bring these up during Cirq Cynque labels Aug 3, 2022
@saeubank
Copy link

saeubank commented Aug 7, 2022

Would this be a good first issue to look into. If so could this be assigned to me?

@daxfohl daxfohl mentioned this issue Nov 5, 2022
@daxfohl
Copy link
Contributor

daxfohl commented Nov 5, 2022

Kinda think this is not possible. Presumably it should work for even exponents too. However to work for even exponents, the gate needs to transform to cirq.I. However that ends up causing all sorts of downstream problems (even after making cirq.I an EigenGate) as can be seen in the linked PR above because cirq.I is special cased so much. Additionally it breaks all the eval(repr(x)) == x etc. protocol tests.

@daxfohl
Copy link
Contributor

daxfohl commented Nov 5, 2022

Well, it's possible, as I managed to get the linked PR to pass all tests. The changes are probably more breaking than desired though. And we need to add some extra handling and tests since IdentityGate, as an EigenGate, can now have a phase attached.

It also leaves Xa*Xa != Xa**2, which seems wrong. I think to solve this completely we'd need a _PauliI (which may be reasonable? Not sure.)

@daxfohl
Copy link
Contributor

daxfohl commented Nov 5, 2022

Nvm, different approach worked.

@daxfohl
Copy link
Contributor

daxfohl commented Nov 6, 2022

Nvm again. The second approach had an issue with PauliString(X(q)**2) turning into PauliString() and then losing the qubit and thus changing the commutation properties. To fix that we'd need to make I a paulistring option, which then we'd have to get into making a _PauliI, and then making Identity an EigenGate like in the first attempt. Doable, but a big change that needs some back-and-forth design and approvals since its backwards compatibility is questionable.

Would be a good issue though for someone with more time to contribute.

@rodolfocarobene
Copy link
Contributor

Hi, I would like to try to work on this issue :-)

@MZuhairKhan
Copy link

Hey there, I am interested in this bounty. Could you assign it to me?

@NoureldinYosri
Copy link
Collaborator

@MZuhairKhan sorry, @rodolfocarobene is already working on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/paulis kind/bug-report Something doesn't seem to work. triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add unitaryhack
Projects
None yet
7 participants