Add reverse permutation for LNN connectivity #12181
Conversation
ShellyGarion
added
Changelog: New Feature
|
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 8865321376Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
| qc_rev = synth_permutation_reverse_lnn_kms(num_qubits) | ||
| qc = qc.compose(qc_rev) |
There was a problem hiding this comment.
This looks good, but I think we can do this more efficiently. While the current implementation using compose ensures correctness, it might not be the most efficient in terms of performance and memory usage, especially for larger circuits.
Instead of composing two quantum circuits, we could consider enhancing efficiency by adding a function that directly appends the reverse permutation LNN to an existing circuit. This approach would modify the circuit in place, avoiding the overhead associated with creating and composing a new circuit instance. Here's a conceptual sketch of how this could be implemented:
def append_reverse_permutation_lnn_kms(qc: QuantumCircuit, num_qubits: int) -> None:
for _ in range((num_qubits + 1) // 2):
_append_cx_stage1(qc, num_qubits)
_append_cx_stage2(qc, num_qubits)
if (num_qubits % 2) == 0:
_append_cx_stage1(qc, num_qubits)There was a problem hiding this comment.
that's a good suggestion, I added it as a helper function.
| def _append_cx_stage1(qc, n): | ||
| """A single layer of CX gates.""" | ||
| for i in range(n // 2): | ||
| qc.cx(2 * i, 2 * i + 1) | ||
| for i in range((n + 1) // 2 - 1): | ||
| qc.cx(2 * i + 2, 2 * i + 1) | ||
| return qc | ||
|
|
||
|
|
||
| def _append_cx_stage2(qc, n): | ||
| """A single layer of CX gates.""" | ||
| for i in range(n // 2): | ||
| qc.cx(2 * i + 1, 2 * i) | ||
| for i in range((n + 1) // 2 - 1): | ||
| qc.cx(2 * i + 1, 2 * i + 2) | ||
| return qc |
There was a problem hiding this comment.
I noticed _append_cx_stage1 and _append_cx_stage2 functions were moved from qiskit/synthesis/linear_phase/cz_depth_lnn.py to qiskit/synthesis/permutation/permutation_reverse_lnn.py. Could you share the reasoning behind this change?
While I understand these are private functions and the move does not impact their functionality, I am curious if the new location makes more sense and better aligns with their specific use.
There was a problem hiding this comment.
These functions are helper functions to construct a reverse permutation, and this is the reason that they have been moved. CZ circuit synthesis for LNN is basically constructing a reverse permutation with phase gates in between.
| @data(4, 5, 10, 15, 20) | ||
| def test_synth_permutation_reverse_lnn_kms(self, num_qubits): |
There was a problem hiding this comment.
Great work on the implementation and testing. The current tests effectively assess the basic functionality and correct usage scenarios. However, I suggest adding tests that focus on:
- Edge Cases: Like scenarios with 0 or 1 qubits.
- Invalid Input: To ensure the system handles invalid inputs as expected.
There was a problem hiding this comment.
I added test cases for 1,2 and 3 qubits.
Summary
See #9036
Add reverse permutations in depth 2n+2 based on Section 5 of Kutin et al. (https://arxiv.org/abs/quant-ph/0701194)
Details and comments