All single-qubit Cliffords #5584
Conversation
cirq-core/cirq/ops/clifford_gate.py
Outdated
| """A metaclass used to lazy initialize several common Clifford Gate as class attributes.""" | ||
|
|
||
| @property | ||
| def all_single_qubit_cliffords(cls): |
There was a problem hiding this comment.
nit: add return type annotation.
cirq-core/cirq/ops/clifford_gate.py
Outdated
| mY = (pauli_gates.Y, True) | ||
| pZ = (pauli_gates.Z, False) | ||
| mZ = (pauli_gates.Z, True) | ||
| cls._all_single_qubit_cliffords = tuple([ |
There was a problem hiding this comment.
nit: use tuple literal instead:
| cls._all_single_qubit_cliffords = tuple([ | |
| cls._all_single_qubit_cliffords = ( |
There was a problem hiding this comment.
Hehe, done before I saw the comment :-)
| if getattr(cls, '_I', None) is None: | ||
| cls._I = cls._generate_clifford_from_known_gate(1, identity.I) | ||
| return cls._I | ||
| return cls.all_single_qubit_cliffords[0] |
There was a problem hiding this comment.
I'm not stoked about these indices which could get out of sync with the tuple of all gates, but I don't have a great alternative. At very least, we should document that the order of the all gates tuple should not be changed without updating these indices.
There was a problem hiding this comment.
Also not a big fan of the constants. An alternative I considered was to use a Dict[str, ...], but it involves making up names for all Clifford gates most of which would be highly non-standard.
I imagine that anyone wanting access to a specific gate would use a property (e.g. CliffordGate.X_sqrt) and folks would use all_single_qubit_cliffords if they really want to get all 24. Here's to wishful thinking :-)
There was a problem hiding this comment.
Perhaps a namedtuple could help here. You could use property names for the items that should be accessible as properties and a say repeated _12 name with the rename=True argument for all other gates for which you don't care about the name.
There was a problem hiding this comment.
AFAICT, the primary (only?) use-case for all_single_qubit_cliffords is the situation where someone wants to iterate over all single-qubit Cliffords, e.g. to execute a test-case on all of them. I think using namedtuple adds unnecessary clutter to output and is potentially confusing.
| if getattr(cls, '_I', None) is None: | ||
| cls._I = cls._generate_clifford_from_known_gate(1, identity.I) | ||
| return cls._I | ||
| return cls.all_single_qubit_cliffords[0] |
maffoo
left a comment
maffoo
left a comment
There was a problem hiding this comment.
Minor comments, then LGTM.
cirq-core/cirq/ops/clifford_gate.py
Outdated
| t = qis.CliffordTableau(num_qubits=2) | ||
| t.xs = [[1, 1], [0, 1], [0, 0], [0, 0]] | ||
| t.zs = [[0, 0], [0, 0], [1, 0], [1, 1]] | ||
| cls._CNOT = cls.from_clifford_tableau(t) |
There was a problem hiding this comment.
For two-qubit cliffords, I'd suggest explicitly calling the method on CliffordGate, since this will be mixed in to the single-qubit class as well:
| cls._CNOT = cls.from_clifford_tableau(t) | |
| cls._CNOT = CliffordGate.from_clifford_tableau(t) |
Same for CZ and SWAP below.
There was a problem hiding this comment.
Thanks for catching this! Done.
| ) | ||
| cls._Z_sqrt = SingleQubitCliffordGate.from_clifford_tableau(_clifford_tableau) | ||
| return cls._Z_sqrt | ||
| return cls.all_single_qubit_cliffords[6] |
There was a problem hiding this comment.
This is the same as S on line 234. Not sure which one is a typo.
Can you perhaps add an independent test that class-property gates have correct values?
If you keep the current property implementation with indexed lookup, such test will fail when
all_single_qubit_cliffords go out of order.
There was a problem hiding this comment.
The S gate and Z_sqrt are the same gate. Square the S gate to convince yourself of that.
The PR does include independent test (test_all_single_qubit_clifford_unitaries) that checks all the gates have correct values. It is already order-sensitive.
pavoljuhas
left a comment
pavoljuhas
left a comment
There was a problem hiding this comment.
S and Z_sqrt are identical, perhaps a typo.
4 participants
This makes it easier to use any single-qubit Clifford gate in tests, including currently neglected order-3 Cliffords. See context.
The PR also simplifies the construction of the gates. In particular, we don't need a simulator to get a gate's tableau.