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

Adds BasisTanslator, UnrollCustomDefinitions to default levels. #4446

Merged
merged 8 commits into from
Jun 25, 2020

Conversation

kdk
Copy link
Member

@kdk kdk commented May 13, 2020

Resolves #3146
Resolves #3086

Summary

  • Fix free param calculation in standard gates to_matrix test.

Previous code mis-calculated the number of available free parameters
from the gate signature, resulting in creation of gates like
XGate(label=0.1). These would cause errors if the circuits containing
these gates were drawn.

  • Remove definition for C3XGate, C4XGate due to name conflict with MCXGate.

  • Add Rz->RX,RY, U3->RX,RY, CX->RXX to StandardEquivalenceLibrary.

Updates BasisTranslator-Unroller compatability test
test_basis_translator.TestUnrollerCompatibility.test_unroll_all_instructions
to verify the generated circuit is equivalent (with snapshot,
measure instructions removed) as the circuit transpiled by the
BasisTranslator is no longer gate-for-gate equal with that of the
Unroller.

  • Add SynthesizeUnitaries and BasisTranslator into default levels.

on-hold for #4442 .

Details and comments

@kdk kdk added on hold Can not fix yet Changelog: New Feature Include in the "Added" section of the changelog labels May 13, 2020
@kdk kdk added this to the 0.15 milestone May 13, 2020
@kdk kdk force-pushed the add-basistranslator-to-default-levels branch 2 times, most recently from 36650e9 to 9d022a9 Compare May 13, 2020 21:59
@ajavadia ajavadia removed the on hold Can not fix yet label May 23, 2020
@ajavadia
Copy link
Member

ajavadia commented May 23, 2020

Does this resolve #3146 and #3086? I think if we add a CX -> CZ and CX -> iSWAP then we have enabled some basic common basis for now.

@kdk kdk force-pushed the add-basistranslator-to-default-levels branch from 9d022a9 to 381ba6b Compare June 2, 2020 21:45
@kdk kdk requested a review from a team as a code owner June 2, 2020 21:45
@kdk
Copy link
Member Author

kdk commented Jun 2, 2020

Does this resolve #3146 and #3086? I think if we add a CX -> CZ and CX -> iSWAP then we have enabled some basic common basis for now.

This resolves #3146, and most of #3086 (except for generalizing commutative cancellation).

@ajavadia
Copy link
Member

Ok I marked it as resolving those two issues (generalizing commutative cancellation seems like a separate issue)

Can you add tests for rewriting some general circuits in terms of CZ and iSWAP?

Copy link
Member

@ajavadia ajavadia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made 2 comments otherwise I'm good with this.

@kdk kdk force-pushed the add-basistranslator-to-default-levels branch 2 times, most recently from 0f45556 to 1d2a89d Compare June 12, 2020 19:21
@kdk kdk force-pushed the add-basistranslator-to-default-levels branch from 1d2a89d to b430cd0 Compare June 12, 2020 20:25
Copy link
Member

@ajavadia ajavadia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since alternate basis are now supported by preset passmanagers, I think they should be supported by individual passes inside those passmanagers too - not just the two newly added passes. For example level 3 is supposed to do block collection and resynthesis to avoid redundant applications of 2-qubit gates. At the moment, however, I think this really just works for CNOTs.

Running this code:

from qiskit.circuit.library import QuantumVolume
from qiskit import transpile
qv_model = QuantumVolume(3, seed=3)
qv_iswap = transpile(qv_model, basis_gates=['iswap', 'u3'], optimization_level=3)
qv_iswap.draw('mpl', fold=100)

I get the circuit below, but all those iswap chains of 6 should be reduced to at most 3 using the block collection/resynthesis passes.

image

@ajavadia
Copy link
Member

This looks good. For 2-qubit gates I think this is doing a reasonable job, but for 1-qubit gates there are still inefficiencies. I fix this as part of #3658

from qiskit import *
qc = QuantumCircuit(2)
qc.cx(0, 1)
qc.cx(1, 0)

out = transpile(qc, basis_gates=['iswap', 'rx', 'rz'], optimization_level=3)

image

kdk added 5 commits June 24, 2020 14:55
Previous code mis-calculated the number of available free parameters
from the gate signature, resulting in creation of gates like
XGate(label=0.1). These would cause errors if the circuits containing
these gates were drawn.
Updates BasisTranslator-Unroller compatability test
test_basis_translator.TestUnrollerCompatibility.test_unroll_all_instructions
to verify the generated circuit is equivalent (with snapshot,
measure instructions removed) as the circuit transpiled by the
BasisTranslator is no longer gate-for-gate equal with that of the
Unroller.
@kdk kdk force-pushed the add-basistranslator-to-default-levels branch from 5d8ab7e to 9727571 Compare June 24, 2020 22:40
@kdk kdk force-pushed the add-basistranslator-to-default-levels branch from 4f8d2f8 to be0e644 Compare June 25, 2020 15:06
@mergify mergify bot merged commit 93a51c8 into Qiskit:master Jun 25, 2020
faisaldebouni pushed a commit to faisaldebouni/qiskit-terra that referenced this pull request Aug 5, 2020
…it#4446)

* Add Rz->RX,RY, U3->RX,RY, CX->RXX to StandardEquivalenceLibrary.

Updates BasisTranslator-Unroller compatability test
test_basis_translator.TestUnrollerCompatibility.test_unroll_all_instructions
to verify the generated circuit is equivalent (with snapshot,
measure instructions removed) as the circuit transpiled by the
BasisTranslator is no longer gate-for-gate equal with that of the
Unroller.

* Fix free param calculation in standard gates to_matrix test.

Previous code mis-calculated the number of available free parameters
from the gate signature, resulting in creation of gates like
XGate(label=0.1). These would cause errors if the circuits containing
these gates were drawn.

* Remove definition for C3XGate, C4XGate due to name conflict with MCXGate.

* Add UnrollCustomDefinitions and BasisTranslator into default levels.

* Add CX to CZ, to iSwap decompositions.

* Support block collection for non-CX two-qubit gates.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: New Feature Include in the "Added" section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC] Unrolling Allow other basis gate sets (e.g. ones which include iSWAP instead of CX)
2 participants