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

Use Optimize1qGatesDecompositon by default in more situations #5531

Closed
mtreinish opened this issue Dec 15, 2020 · 3 comments
Closed

Use Optimize1qGatesDecompositon by default in more situations #5531

mtreinish opened this issue Dec 15, 2020 · 3 comments
Assignees
Labels
type: enhancement It's working, but needs polishing
Milestone

Comments

@mtreinish
Copy link
Member

What is the expected enhancement?

We have 2 passes that do 1q gate optimization Optimize1qGates which only works on u1, u2, u3 (and u and p which are identical to u3 and u) and Optimize1qGatesDecomposition which works with any basis. Right now all the default pass managers are using logic to pick between the two passes with:

    if basis_gates and ('u1' in basis_gates or 'u2' in basis_gates or
                        'u3' in basis_gates):
        _opt = [Optimize1qGates(basis_gates), CXCancellation()]
    else:
        _opt = [Optimize1qGatesDecomposition(basis_gates), CXCancellation()]

This is less than ideal because for over complete basis that include u1, u2, or u3 the resulting output is not fully optimized. We should be using the Optimize1qGatesDecomposition by default; except for where the 1q component of a basis set consists solely of u, p, u1, u2, or u3. This is because while after #5468 is fixed Optimize1qGatesDecomposition will produce the same result as Optimize1qGates in all cases for that case Optimize1qGatesDecomposition it's significantly slower than Optimize1qGates so we should use the faster pass in that case.

When this is implemented it will also fix #5436

@mtreinish mtreinish added the type: enhancement It's working, but needs polishing label Dec 15, 2020
@mtreinish mtreinish added this to the 0.17 milestone Dec 15, 2020
@enavarro51
Copy link
Contributor

@mtreinish and @kdk, combining #5468 and this issue, here's what I believe has to be done.

  1. Add U3 to U2 or U1 and similar for U reductions in OneQubitEulerDecomposer.
  2. In Optimize1qGatesDecomposition.run, allow single 1q gate optimization if the gate is U or U3, so that item 1 will go through the optimization.
  3. Change the 'if' in the code above in level1, 2, and 3 to check that all 1q components of the basis are in ['u', 'p', 'u1', 'u2', 'u3'] and if so continue to use Optimize1qGates for this case. No changes to Optimize1qGates are required except change this line in __init__, self.basis = basis if basis else ["u1", "u2", "u3"] to self.basis = basis if basis else ["u", "p", "u1", "u2", "u3"]
  4. Leave all test_optimize_1q_gates tests as is.
  5. Add tests to test_optimize_1q_decomposition to test that the U3 and U reductions work.

@kdk
Copy link
Member

kdk commented Dec 16, 2020

@enavarro51 The above plan looks good. (Though for 3, I don't know that it's straightforward to distinguish the 1q from 2q gates in basis gates. It may be better to split out the pass manager changes into a separate PR.) Feel free to reach out with any questions.

@kdk
Copy link
Member

kdk commented Mar 4, 2021

This was covered by #5554 .

@kdk kdk closed this as completed Mar 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement It's working, but needs polishing
Projects
None yet
Development

No branches or pull requests

3 participants