Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add optional restore_layout flag to transpiler #4608

Open
mlxd opened this issue Jun 23, 2020 · 10 comments
Open

Add optional restore_layout flag to transpiler #4608

mlxd opened this issue Jun 23, 2020 · 10 comments
Assignees
Labels
type: feature request New feature or request

Comments

@mlxd
Copy link
Contributor

mlxd commented Jun 23, 2020

Information

  • Qiskit Terra version: 0.14.1
  • Python version: 3.7.7
  • Operating system: MacOS Catalina

What is the current behavior?

The circuit as defined in ncx.qasm (below) produces different results after transpilation with the BasicSwap pass. Running the circuit without transpilation on the qasm_simulator backend produces an output of '100011111', and '110101011' with transpilation.

Steps to reproduce the problem

transpiler_err.tar.gz

The included transpiler_err.tar.gz has files transpiler_err.py and ncx.qasm, which demonstrate the outputs from the non-transplied and transpiled circuits. Run the example as:

tar xvf transpiler_err.tar.gz && cd ./transpiler_err
python transpiler_err.py

This will produce the outputs:
{'100011111': 1000} {'110101011': 1000}

What is the expected behavior?

For the above example, the output's should be
{'100011111': 1000} {'100011111': 1000}

Suggested solutions

Not currently sure. The work this example falls under is part of the QuantEx project, run by @nmoran who may add more details and suggestions.

I will also list the explicit contents of the above files below:

transpiler_err.py is defined as

from qiskit import QuantumCircuit
from qiskit import execute
from qiskit import Aer

from qiskit.compiler import transpile
from qiskit.transpiler import PassManager
import qiskit.transpiler as transpiler

import qiskit.transpiler.passes as passes

circ = QuantumCircuit.from_qasm_file("ncx.qasm")
circ = circ.decompose()

coupling = [[i-1, i] for i in range(1, circ.num_qubits)]
coupling_map = transpiler.CouplingMap(coupling)
p = passes.BasicSwap(coupling_map=coupling_map)


pass_manager = transpiler.PassManager(p)
circ_t = pass_manager.run(circ)

circ.measure(range(9), range(9))
circ_t.measure(range(9), range(9))

simulator = Aer.get_backend('qasm_simulator')

# Execute the circuit on the qasm simulator
job = execute(circ, simulator, shots=1000)
job_t = execute(circ_t, simulator, shots=1000)

# Grab results from the job
result = job.result()
result_t = job_t.result()

# Returns counts
counts = result.get_counts(circ)
counts_t = result_t.get_counts(circ_t)

print(counts, counts_t)

and ncx.qasm is defined as

OPENQASM 2.0;
include "qelib1.inc";
qreg q[9];
creg c[9];
gate asx tgt{
    u3(1.5707963267948966,1.5707963267948966,-1.5707963267948966) tgt;
}
gate c_asx ctrl,tgt{
    cu3(1.5707963267948966,1.5707963267948966,-1.5707963267948966) ctrl,tgt;
}
gate sz tgt{
    u3(0.0,0.7853981633974484,0.7853981633974484) tgt;
}
gate c_sz ctrl,tgt{
    cu3(0.0,0.7853981633974484,0.7853981633974484) ctrl,tgt;
}
gate asz tgt{
    u3(0.0,-0.7853981633974484,-0.7853981633974484) tgt;
}
gate c_asz ctrl,tgt{
    cu3(0.0,-0.7853981633974484,-0.7853981633974484) ctrl,tgt;
}
gate sx tgt{
    u3(1.5707963267948966,-1.5707963267948966,1.5707963267948966) tgt;
}
gate c_sx ctrl,tgt{
    cu3(1.5707963267948966,-1.5707963267948966,1.5707963267948966) ctrl,tgt;
}
x q[0];
x q[1];
x q[2];
x q[3];
x q[4];
h q[8];
c_sz q[7],q[8];
h q[8];
cx q[4],q[7];
h q[8];
c_asz q[7],q[8];
h q[8];
cx q[4],q[7];
h q[8];
c_sz q[4],q[8];
h q[8];
h q[7];
c_sz q[6],q[7];
h q[7];
cx q[3],q[6];
h q[7];
c_asz q[6],q[7];
h q[7];
cx q[3],q[6];
h q[7];
c_sz q[3],q[7];
h q[7];
h q[6];
c_sz q[5],q[6];
h q[6];
cx q[2],q[5];
h q[6];
c_asz q[5],q[6];
h q[6];
cx q[2],q[5];
h q[6];
c_sz q[2],q[6];
h q[6];
h q[5];
c_sz q[1],q[5];
h q[5];
cx q[0],q[1];
h q[5];
c_asz q[1],q[5];
h q[5];
cx q[0],q[1];
h q[5];
c_sz q[0],q[5];
h q[5];
h q[6];
c_sz q[5],q[6];
h q[6];
cx q[2],q[5];
h q[6];
c_asz q[5],q[6];
h q[6];
cx q[2],q[5];
h q[6];
c_sz q[2],q[6];
h q[6];
h q[7];
c_sz q[6],q[7];
h q[7];
cx q[3],q[6];
h q[7];
c_asz q[6],q[7];
h q[7];
cx q[3],q[6];
h q[7];
c_sz q[3],q[7];
h q[7];
h q[8];
c_sz q[7],q[8];
h q[8];
cx q[4],q[7];
h q[8];
c_asz q[7],q[8];
h q[8];
cx q[4],q[7];
h q[8];
c_sz q[4],q[8];
h q[8];
h q[7];
c_sz q[6],q[7];
h q[7];
cx q[3],q[6];
h q[7];
c_asz q[6],q[7];
h q[7];
cx q[3],q[6];
h q[7];
c_sz q[3],q[7];
h q[7];
h q[6];
c_sz q[5],q[6];
h q[6];
cx q[2],q[5];
h q[6];
c_asz q[5],q[6];
h q[6];
cx q[2],q[5];
h q[6];
c_sz q[2],q[6];
h q[6];
h q[5];
c_sz q[1],q[5];
h q[5];
cx q[0],q[1];
h q[5];
c_asz q[1],q[5];
h q[5];
cx q[0],q[1];
h q[5];
c_sz q[0],q[5];
h q[5];
h q[6];
c_sz q[5],q[6];
h q[6];
cx q[2],q[5];
h q[6];
c_asz q[5],q[6];
h q[6];
cx q[2],q[5];
h q[6];
c_sz q[2],q[6];
h q[6];
h q[7];
c_sz q[6],q[7];
h q[7];
cx q[3],q[6];
h q[7];
c_asz q[6],q[7];
h q[7];
cx q[3],q[6];
h q[7];
c_sz q[3],q[7];
h q[7];
@mlxd mlxd added the bug Something isn't working label Jun 23, 2020
@kdk
Copy link
Member

kdk commented Jun 23, 2020

Hi @mlxd , thanks for reporting this.

pass_manager = transpiler.PassManager(p)
circ_t = pass_manager.run(circ)

circ.measure(range(9), range(9))
circ_t.measure(range(9), range(9))

Can you try appending the measure to circ prior to running the pass manager? BasicSwap (and the other routing passes) will in general permute the final layout of qubits during routing. If measurements are present when the routing pass is run, they will be updated so the measurement values in the output counts will be consistent with the input circuit.

@mlxd
Copy link
Contributor Author

mlxd commented Jun 24, 2020

H @kdk thanks for the suggestion. I can confirm that adding the measurement gates prior to the pass does fix the ordering. Though, the intended use for this circuit will be for feed into another stage, rather than a measurement at this point; the measurement here was more of a demonstration of the results we observed rather than full intended use.

Do you think it possible to obtain and indicate this permuted ordering without the measurement? Or, if not, whether putting the measurement in and popping them from the circuit after the pass makes sense? Thanks again for all assistance with this.

@ajavadia
Copy link
Member

You are right that mapping sub-circuits is currently not supported. This is an open issue to permute the qubits back to where they were. We have code for doing this (TokenSwap #3762) but it has not been integrated in the default flow yet.

@nmoran
Copy link

nmoran commented Jun 29, 2020

Thanks for the update @ajavadia. For now we have implemented a workaround where we add measurements for each qubit, apply the transpilation pass (basic swap), read the final mapping of qubits from the measurement operations and finally remove the measurement operations. This is working well so far for the cases we have tried but will look into TokenSwap if we need something more robust.

@1ucian0
Copy link
Member

1ucian0 commented Jul 2, 2020

Should this issue be closed?

@mlxd
Copy link
Contributor Author

mlxd commented Jul 6, 2020

Hi @1ucian0 If there is an existing task to follow the integration of the TokenSwap work to solve our above issue I can close this and follow that work instead.

Though, happy to leave it at your discretion.

@ajavadia
Copy link
Member

I'm not sure there's another issue. This should be closed after this issue is fixed. It is simple: just call the LayoutTransformation pass after routing is done. This will restore the original layout efficiently. I think this needs a transpile(restore_layout=True) option though, it should be opt-in.

@kdk kdk changed the title Different results from circuit when transpiling with BasicSwap Add optional restore_layout flag to transpiler Jul 23, 2020
@kdk kdk added type: feature request New feature or request and removed bug Something isn't working labels Jul 23, 2020
@1ucian0 1ucian0 self-assigned this Jul 24, 2020
@1ucian0
Copy link
Member

1ucian0 commented Aug 21, 2020

This problem is related to #4911

The first step is to have a preserve_statevector (see #4911 (comment)) conflicting with coupling_map and initial_layout. Then we are going to introduce a pass to restore the layout to remove that conflict. We need to have a list of the swaps introduced to by the swappers in orden to undo them.

@ajavadia
Copy link
Member

ajavadia commented Aug 22, 2020 via email

@1ucian0
Copy link
Member

1ucian0 commented Aug 24, 2020

I dont think we need to undo swaps. We should use the token swapper pass to take one layout to another. It can be much more efficient than undoing swaps (because the swaps were dictated by the pattern of gates).

This is definitely a possible approach. Consider that the token swapper pass also needs to take a coupling map as an argument.

Also not sure a preserve_statevector flag is the right approach. I think we should always preserve statevector. Just need a pass to clean up extra swaps if they appear before measurements (absorb them into the measurements and retarget them).

Let's move this conversation about preserve_statevector to #4911

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature request New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants