Refactor the single qubit Clifford gate #5069
Conversation
This reverts commit b2d9269.
cirq-core/cirq/ops/clifford_gate.py
Outdated
| @property | ||
| def X(cls): | ||
| if getattr(cls, '_X', None) is None: | ||
| cls._Z = cls._generate_clifford_from_known_gate(1, pauli_gates.X) |
There was a problem hiding this comment.
Are these copy paste errors? The _Xs, _Ys, and _Zs are all intermixed.
There was a problem hiding this comment.
Yes, great catch! Thanks!
| @@ -878,3 +490,387 @@ def _act_on_(self, args: 'cirq.ActOnArgs', qubits: Sequence['cirq.Qid']) -> bool | |||
| return NotImplemented | |||
|
|
|||
| return NotImplemented | |||
|
|
|||
There was a problem hiding this comment.
While you're here, the sig for _act_on_ should be _act_on_(self, args: 'cirq.OperationTarget', qubits: Sequence['cirq.Qid']) -> bool
There was a problem hiding this comment.
Sure, will update it in this PR as well.
MichaelBroughton
added
the
automerge
|
Automerge cancelled: There are merge conflicts. |
CirqBot
removed
the
automerge
cirq-core/cirq/ops/clifford_gate.py
Outdated
| args: 'cirq.OperationTarget', # pylint: disable=unused-argument | ||
| qubits: Sequence['cirq.Qid'], # pylint: disable=unused-argument | ||
| ): | ||
| # TODO(PR number) will create a PR if we agree on adding this. |
There was a problem hiding this comment.
@daxfohl @MichaelBroughton Do you have any objection on this?
There was a problem hiding this comment.
No, but thanks for asking. I noticed this a few weeks ago and my initial thought was to ask for a change here. I'd been trying to remove all the isinstance checks from act_on because it made more sense that the simulator should know about gates but not vice versa. However here, I feel like this is an example of a gate that operates on a higher level than the simulator, because the gate itself is specifically designed to work with Clifford simulators. So I think this is an example where it's okay.
There was a problem hiding this comment.
None from me. Can we resolve the merge conflicts so we can retry the auto merge ?
|
Looks like there is still a little bit of formatting and merge conflicts. |
|
Yeah, that probably is because of recent black version update. Hopefully, it is resolved this time. |
MichaelBroughton
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
CirqBot
removed
automerge
This PR mainly moved the `SingleQubitCliffordGate` as the derived class of `CliffordGate`. It didn't add any new functionalities or features. The code change looks large but actually not. Because I moved the `SingleQubitCliffordGate` to the place after `CliffordGate` and auto-diff is not that smart enough to tell that. Context quantumlib#4791. Main change are: 1. Modify the value equality so that SingleQubitCliffordGate can be evaluate equal to the same CliffordGate 2. Moved the module-level initialization common SingleQubitCliffordGate (like X, Y, Z) to the class-level one. 3. Make a few function `SingleQubitCliffordGate` to use `CliffordGate` one.
This PR mainly moved the `SingleQubitCliffordGate` as the derived class of `CliffordGate`. It didn't add any new functionalities or features. The code change looks large but actually not. Because I moved the `SingleQubitCliffordGate` to the place after `CliffordGate` and auto-diff is not that smart enough to tell that. Context quantumlib#4791. Main change are: 1. Modify the value equality so that SingleQubitCliffordGate can be evaluate equal to the same CliffordGate 2. Moved the module-level initialization common SingleQubitCliffordGate (like X, Y, Z) to the class-level one. 3. Make a few function `SingleQubitCliffordGate` to use `CliffordGate` one.
This PR mainly moved the `SingleQubitCliffordGate` as the derived class of `CliffordGate`. It didn't add any new functionalities or features. The code change looks large but actually not. Because I moved the `SingleQubitCliffordGate` to the place after `CliffordGate` and auto-diff is not that smart enough to tell that. Context quantumlib#4791. Main change are: 1. Modify the value equality so that SingleQubitCliffordGate can be evaluate equal to the same CliffordGate 2. Moved the module-level initialization common SingleQubitCliffordGate (like X, Y, Z) to the class-level one. 3. Make a few function `SingleQubitCliffordGate` to use `CliffordGate` one.
This PR mainly moved the
SingleQubitCliffordGateas the derived class ofCliffordGate.It didn't add any new functionalities or features. The code change looks large but actually not. Because I moved the
SingleQubitCliffordGateto the place afterCliffordGateand auto-diff is not that smart enough to tell that. Context #4791.Main change are:
SingleQubitCliffordGateto useCliffordGateone.