fix skip_transpiler: do not remove gates #562
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
atilag
approved these changes
Jun 16, 2018
| @@ -126,9 +126,11 @@ def compile(circuits, backend, | |||
| if skip_transpiler: # Just return the qobj, without any transformation or analysis | |||
| job["config"]["layout"] = None | |||
| job["compiled_circuit_qasm"] = circuit.qasm() | |||
| basis_all = "id,u0,u1,u2,u3,x,y,z,h,s,sdg,t,tdg,rx,ry,rz," \ | |||
| "cx,cy,cz,ch,crz,cu1,cu3,swap,ccx,cswap" | |||
There was a problem hiding this comment.
I remember seeing many other places where we have similar basis gates definition hardcoded as well, I guess we could review this in the near future and find out a way to have a unique source for it
There was a problem hiding this comment.
@atilag yes, this definitely has to change. It's ugly and bugs like this pop up. It's on my list to go through all the code and change this. But that's for a future PR.
ewinston
reviewed
Jun 18, 2018
| @@ -126,9 +126,11 @@ def compile(circuits, backend, | |||
| if skip_transpiler: # Just return the qobj, without any transformation or analysis | |||
| job["config"]["layout"] = None | |||
| job["compiled_circuit_qasm"] = circuit.qasm() | |||
| basis_all = "id,u0,u1,u2,u3,x,y,z,h,s,sdg,t,tdg,rx,ry,rz," \ | |||
There was a problem hiding this comment.
Maybe we can avoid defining basis_all by doing,
if skip_transpiler: # Just return the qobj, without any transformation or analysis
job["config"]["layout"] = None
dag_circuit = DAGCircuit.fromQuantumCircuit(circuit)
job["compiled_circuit_qasm"] = dag_circuit.qasm(qeflag=True,
eval_symbols=True)
job["compiled_circuit"] = DagUnroller(dag_circuit,
JsonBackend(dag_circuit.basis)).execute()
ewinston
added a commit
to ewinston/qiskit
that referenced
this pull request
Jun 18, 2018
in pr Qiskit#562 a basis string constant was defined which seems like could be avoided.
ajavadia
pushed a commit
that referenced
this pull request
Jun 18, 2018
in pr #562 a basis string constant was defined which seems like could be avoided.
lia-approves
pushed a commit
to edasgupta/qiskit-terra
that referenced
this pull request
Jul 30, 2019
…-unroll fix skip_transpiler: do not remove gates
lia-approves
pushed a commit
to edasgupta/qiskit-terra
that referenced
this pull request
Jul 30, 2019
in pr Qiskit#562 a basis string constant was defined which seems like could be avoided.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
The
skip_transpilerflag was silently removing gates that did not match a backend's basis set. This PR fixes that.Details and comments