Add default decomposition for cirq.QubitPermutationGate in terms of adjacent swaps #5093
Conversation
| for i in range(len(qubits)): | ||
| q_i = qubit_ids.index(self._permutation.index(i)) | ||
| for j in range(q_i, i, -1): | ||
| yield swap_gates.SWAP(qubits[j], qubits[j - 1]) |
There was a problem hiding this comment.
Limiting SWAPs to adjacent-index qubits presupposes a linear qubit layout; on a 2D grid or more exotic layout, this will not work:
q = GridQubit.rect(2, 2)
# q[1] and q[2] are not adjacent ((0, 1) and (1, 0))
There was a problem hiding this comment.
Note that the decomposition here is not intended to be architecture aware. The goal here is to simply write some decomposition in terms of other "simpler" gates which eventually ends up decomposing into the native gateset.
For device specific routing, we would need more sophisticated transformers which take care of device topology, but that is out of scope of this PR. The choice to use adjacent index swaps which can be useful for a linear qubit layout is an arbitrary one, mostly because running circuits on a chain of qubits is a common use case and a low hanging fruit.
There was a problem hiding this comment.
If this is meant to be architecture-agnostic, why don't we swap qubits directly (ignoring adjacency)?
There was a problem hiding this comment.
As I said, it's a choice we need to make. Both the approaches would be valid for the scope of this PR.
If a user is relying on the default decomposition of Cirq for compiling their circuit, it means they have already given up fine grained control. The reason I want to stick to this approach is that it's a low hanging fruit which targets a common use case of running circuits on a chain of qubits. It's not going to be enough for every scenario, but still slightly better than assuming all to all connectivity.
There was a problem hiding this comment.
It's not going to be enough for every scenario, but still slightly better than assuming all to all connectivity.
I'm not convinced that linear-swap is better than all-to-all, or even equally valid. Compared to linear-swap, all-to-all is:
- cheaper to run on simulators (which use default decomposition)
- easier to convert to efficient circuits in device-specific layout
- cheaper to construct here (low priority, but worth considering)
I'd support adding a separate method for converting to linear-swap, but the default should be all-to-all.
There was a problem hiding this comment.
cheaper to run on simulators (which use default decomposition)
The class already has a _apply_unitary_ defined, which should be used by simulators over the default decomposition. Is that not the case?
There was a problem hiding this comment.
This is true for Cirq-internal simulators, but not in general. For example, qsim attempts to decompose gates to a specific subset that it natively supports, falling back on conversion to unitary only if no decomposition is defined (code).
There was a problem hiding this comment.
Opened #5097 to track the optimization.
As mentioned above, the goal here is to just decompose into a set of simpler operations which ends up at the default cirq gateset. Topology aware qubit routing should be handled separately by more sophisticated transformers. |
tanujkhattar
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
CirqBot
removed
automerge
…djacent swaps (quantumlib#5093) - Adds decomposition to `cirq.QubitPermutationGate` in terms of minimum number of adjacent swap operations on qubits. - Part of quantumlib#4858 Closes quantumlib#5090
…djacent swaps (quantumlib#5093) - Adds decomposition to `cirq.QubitPermutationGate` in terms of minimum number of adjacent swap operations on qubits. - Part of quantumlib#4858 Closes quantumlib#5090
…djacent swaps (quantumlib#5093) - Adds decomposition to `cirq.QubitPermutationGate` in terms of minimum number of adjacent swap operations on qubits. - Part of quantumlib#4858 Closes quantumlib#5090
cirq.QubitPermutationGatein terms of minimum number of adjacent swap operations on qubits.Closes #5090