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

Merge u1, u2 and p gate together if they contain parameters. #7309

Closed
wants to merge 13 commits into from
Closed

Merge u1, u2 and p gate together if they contain parameters. #7309

wants to merge 13 commits into from

Conversation

rafal-pracht
Copy link
Contributor

Summary

Before the fix, the 'Optimize1qGates' transpiler optimization join together the gate only when the parameter was bounded (or when there was no parameter at all). After the fix, the u1, u2, and p gates will be combined even when the parameter is unbounded.

from qiskit import QuantumCircuit
from qiskit.circuit import Parameter
from qiskit.transpiler import PassManager
from qiskit.transpiler.passes import Optimize1qGates, Unroller

phi = Parameter('φ')
alpha = Parameter('α')

qc = QuantumCircuit(1)
qc.u1(2*phi, 0)
qc.u1(alpha, 0)
qc.u1(0.1, 0)
qc.u1(0.2, 0)
qc.draw(output='mpl')

image

pm = PassManager([Unroller(['u1', 'cx']), Optimize1qGates()])
nqc = pm.run(qc)
nqc.draw(output='mpl')

This change is a part of the advocate project done under @nbronn.

image

@rafal-pracht rafal-pracht requested a review from a team as a code owner November 24, 2021 12:30
@coveralls
Copy link

coveralls commented Nov 24, 2021

Pull Request Test Coverage Report for Build 1592690401

  • 16 of 19 (84.21%) changed or added relevant lines in 1 file are covered.
  • 5 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.002%) to 83.116%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/transpiler/passes/optimization/optimize_1q_gates.py 16 19 84.21%
Files with Coverage Reduction New Missed Lines %
qiskit/extensions/quantum_initializer/uc.py 2 88.65%
qiskit/pulse/library/waveform.py 3 89.36%
Totals Coverage Status
Change from base Build 1587737912: -0.002%
Covered Lines: 51914
Relevant Lines: 62460

💛 - Coveralls

Copy link
Member

@kdk kdk left a comment

Choose a reason for hiding this comment

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

Thanks for taking this on @rafal-pracht . This looks good so far, a few small comments below.

@rafal-pracht
Copy link
Contributor Author

Thanks @kdk for your feedback, I've made changes.

Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

This LGTM, thanks for the fast updates and sorry this has fallen through the cracks. The only thing missing here is a release note to document the new feature that the pass will work with unbound parameters on u1, u2, and p gates. You can see a guide on how to do this here: https://github.com/Qiskit/qiskit-terra/blob/main/CONTRIBUTING.md#release-notes once there's a release note here I think this is ready to merge

@mtreinish mtreinish added the Changelog: New Feature Include in the "Added" section of the changelog label Jan 25, 2022
@mtreinish mtreinish modified the milestone: 0.20 Jan 25, 2022
@mtreinish
Copy link
Member

Closing this as the fork this PR was created from has been deleted so this PR is now effectively read only and we can't make modifications to the PR code anymore. This has been reopened from a new fork by the author at #7579 and we'll continue the review in that PR.

@mtreinish mtreinish closed this Jan 26, 2022
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.

4 participants