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

Constructing a Clifford from an Instruction discards phase #11330

Closed
samanthavbarron opened this issue Nov 27, 2023 · 5 comments · Fixed by #12177
Closed

Constructing a Clifford from an Instruction discards phase #11330

samanthavbarron opened this issue Nov 27, 2023 · 5 comments · Fixed by #12177
Assignees
Labels
documentation Something is not clear or an error documentation mod: quantum info Related to the Quantum Info module (States & Operators)
Milestone

Comments

@samanthavbarron
Copy link
Contributor

Environment

  • Qiskit Terra version: 0.25.3
  • Python version: 3.10.12
  • Operating system: macOS Ventura 13.6

What is happening?

If you make a Clifford from an Instruction which contains a phase, the phase is discarded.

How can we reproduce the issue?

from qiskit.quantum_info import Pauli, Clifford

Clifford(Pauli("-I").to_instruction()) == Clifford(Pauli("I").to_instruction())
>>> True

What should happen?

I am not sure if this quite qualifies as a bug, but it seems like one of these would be helpful:

  • Return False.
  • Raise an error when the phase of the input instruction is non-zero.
  • Display a warning when the phase of the input instruction is non-zero.
  • Include a statement about this in the documentation somewhere (unless I missed it!).

Any suggestions?

No response

@samanthavbarron samanthavbarron added the bug Something isn't working label Nov 27, 2023
@jakelishman
Copy link
Member

This looks like a straight bug to me - I imagine it's just that we forget to examine the global_phase of instruction definitions when building a Clifford from a circuit or instruction. I think the right course of action will be to succeed if the phase is a multiple of $\pi/2$ and fail if it's not, similar to how the $R_z$ gate (e.g.) is brought in.

@jakelishman jakelishman added the mod: quantum info Related to the Quantum Info module (States & Operators) label Nov 29, 2023
@jakelishman jakelishman added this to the 1.0.0 milestone Nov 29, 2023
@ShellyGarion
Copy link
Member

ShellyGarion commented Jan 3, 2024

I do not think that this is a bug, since a Clifford is only defined up to a global phase. Indeed, it's not written in the documentation, but this is something well known (see e.g. this summary about the Clifford group).

Here are a few examples of Clifford identities:

qc = QuantumCircuit(1)
qc.s(0)
qc.h(0)
qc.s(0)
qc.h(0)
qc.s(0)
qc.h(0)
Clifford(qc) == Clifford(Pauli("I").to_instruction()) # True
Operator(qc).equiv(Operator(Pauli("I").to_instruction())) # True
Operator(qc) == Operator(Pauli("I").to_instruction()) # False

and:

qc1 = QuantumCircuit(1)
qc1.x(0)
qc1.y(0)

qc2 = QuantumCircuit(1)
qc2.z(0)

Clifford(qc1) == Clifford(qc2) # True
Operator(qc1).equiv(Operator(qc2)) # True
Operator(qc1) == Operator(qc2) # False

I would therefore suggest to close this issue.

@jakelishman
Copy link
Member

Oh yeah, you're right Shelly, sorry.

For sure I think we ought to update the documentation to highlight this, since it's a defined part of the group. I think we might want to unify the various construction methods to ensure they behave similarly, though; at the moment, Clifford(Pauli(...)) and Clifford.from_label will both reject Paulis that include an explicit non-zero phase ("-X", etc), but Clifford.from_instruction silently accepts such circuits, as shown in the issue.

I think that making Clifford.from_instruction reject Paulis with non-zero phases could be quite inconvenient for users, so I think keeping the current behaviour of erasing the phases is potentially the best. In that case, it might be helpful to make Clifford.from_label also erase the phase, so that Clifford(Pauli(qc)) and Clifford(qc)` will both accept the same set of circuits.

I don't think that emitting a warning would be great UX, given the class representation explicitly is phase-agnostic because of the equivalent action of the group. If it's required, perhaps we could add a strict_phase kwarg to the constructors that could be used to turn a non-zero phase into an error, though? I can't think of any uses for that, though, so I'd avoid it unless we've got a use-case.

Does that sound reasonable?

@ShellyGarion ShellyGarion added documentation Something is not clear or an error documentation and removed bug Something isn't working labels Jan 7, 2024
@ShellyGarion
Copy link
Member

ShellyGarion commented Jan 7, 2024

I agree that it's more a documentation issue and not a bug, so I changed the label accordingly.
The current documentation does not explain what a Clifford is, and relies on the fact that the users are aware of the literature, so perhaps this can be improved.
Since a Clifford object can be created in so many different ways: from a QuantumCircuit, Instruction, Gate, LinearFunction, Permutation, Pauli etc. (and combinations of all the above), it may be difficult to impose a consistent warning behavior.

@jakelishman
Copy link
Member

Since a Clifford object can be created in so many different ways: from a QuantumCircuit, Instruction, Gate, LinearFunction, Permutation, Pauli etc. (and combinations of all the above), it may be difficult to impose a consistent warning behavior

I don't think we should warn, I was more meaning that we should be consistent in how we handle phases. The Clifford class represents an object that's only defined up to a phase, and things like composition with a Pauli should happily throw away the phase of Pauli term, so I think it makes sense to make sure that Clifford.from_label and all other Clifford constructors treat phase in the same way (i.e. ignore a global phase). Clifford.from_label at the moment just doesn't have the handling in place to throw away the phase, so it chokes.

The current documentation does not explain what a Clifford is, and relies on the fact that the users are aware of the literature, so perhaps this can be improved.

Yeah, I'm roughly familiar with it and I still made the same mistake too, so definitely lets comment about exactly what Clifford represents in its documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Something is not clear or an error documentation mod: quantum info Related to the Quantum Info module (States & Operators)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants