Optimise QubitPermutationGate decomposition

[migueltorrescosta]

Solves #5097 as per comment #5097 (comment) .

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.79%. Comparing base (9ddb8c8) to head (8b5a61f).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6588   +/-   ##
=======================================
  Coverage   97.79%   97.79%           
=======================================
  Files        1126     1126           
  Lines       95784    95781    -3     
=======================================
+ Hits        93670    93671    +1     
+ Misses       2114     2110    -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[NoureldinYosri]

This code has time complexity O(num_qubits * num_cycles) but can be done in O(num_qubits), try

permutation = self.permutation.copy()

for i in range(len(permutation)):
   if permutation[i] == -1: continue
   cycle = [i]
   while permutation[cycle[-1]] != i:
      cycle.append(permutation[cylce[-1]])
   for j in cycle: permutation[j] = -1
   cycle.reverse() 
   # yield len(cycle) - 1 swap operations
   

[migueltorrescosta]

What should if permutation[i] = -1: continue be? It does not compile

[migueltorrescosta]

Letting

• c being the number of cycles
• q being the number of qubits

I believe the complexity is O(q) because the inner loop runs c/q times ( average over all loops ), and the outer loop runs c times ( once per cycle ). The total complexity then becomes O(c * (q/c) ) = O(q) . Did I miss something?

reversed_permutation_map = {v: i for i, v in enumerate(self.permutation)} # q

while reversed_permutation_map:  # This loop runs once per cycle
    a = list(reversed_permutation_map.keys())[0]
    b = reversed_permutation_map.pop(a)
    while b in reversed_permutation_map.keys():  # This loop runs once per qubit in a cycle
        yield swap_gates.SWAP(qubits[a], qubits[b])
        (a, b) = (b, reversed_permutation_map.pop(b))

[NoureldinYosri]

list(reversed_permutation_map.keys())[0]

this line is O(n)

[NoureldinYosri]

you can make your implementation O(q) by changing that line to

a = next(iter(reversed_permutation_map))

but even in that case the implementation would be a bit convoluted

[NoureldinYosri]

all the CI checks pass for current commits, where is this test failing?

[migueltorrescosta]

The tests fail when I attempted the suggested solution #6588 (comment) , even with the fixes mentioned in #6588 (comment). They do not fail with the currently committed code.

[migueltorrescosta]

@NoureldinYosri I believe this is ready to go as is. Do mention it if there is something I should change.

[NoureldinYosri]

please commit the code that fails

[migueltorrescosta]

Done

[migueltorrescosta]

Suggestions applied. The shorter code feels easier to follow:

remaining_permutations = {v: i for i, v in enumerate(self.permutation)}

while remaining_permutations:
    a = next(iter(remaining_permutations))
    b = remaining_permutations.pop(a)
    while b in remaining_permutations.keys():
        yield swap_gates.SWAP(qubits[a], qubits[b])
        (a, b) = (b, remaining_permutations.pop(b))

vs

permutation = [p for p in self.permutation]

for i in range(len(permutation)):

    if permutation[i] == -1:
        continue
    cycle = [i]
    while permutation[cycle[-1]] != i:
        cycle.append(permutation[cycle[-1]])

    for j in cycle:
        permutation[j] = -1

    for idx in cycle[1:]:
        yield swap_gates.SWAP(qubits[cycle[0]], qubits[idx])

It might be a question of personal taste, so I am happy to have either merged

[NoureldinYosri]

thanks for your help with this task

fixes #5097