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 paramterized u1, u2 and p gate in Optimize1qGates #7579

Merged
merged 12 commits into from
Jan 28, 2022
Merged

Merge paramterized u1, u2 and p gate in Optimize1qGates #7579

merged 12 commits into from
Jan 28, 2022

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

This is continuation of the issue #7309

@rafal-pracht rafal-pracht requested a review from a team as a code owner January 26, 2022 14:26
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.

I'm a bit confused by this PR, it looks identical to #7309 except you've added the release note I asked for in my review yesterday. To add the release note there is no need to open a new pull request you can just modify the existing branch for #7309. It's generally better to use the existing PR so the entire review history is present and the merged commit refers to the single place where all the review happened.

@rafal-pracht
Copy link
Contributor Author

@mtreinish you are absolutely right. I'm unable to add release notes to PR because I'm also working on the @nbronn fork of terra and GitHub lost the connection between my code and the last PR. Therefore I've created a new one with exactly the same code with the release notes added.

Sorry for confusion.

@coveralls
Copy link

coveralls commented Jan 26, 2022

Pull Request Test Coverage Report for Build 1762015271

  • 16 of 19 (84.21%) changed or added relevant lines in 1 file are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.001%) to 83.149%

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/pulse/library/waveform.py 3 89.36%
Totals Coverage Status
Change from base Build 1761987769: 0.001%
Covered Lines: 51905
Relevant Lines: 62424

💛 - Coveralls

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.

It looks like some of the revisions made on #7309 didn't make it through the transition to this new PR. I also left some inline improvement suggestions on the release note. Mostly around som sphinx rst syntax fixes and some mild rewording.

rafal-pracht and others added 2 commits January 26, 2022 17:00
…c523251108c.yaml

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
@rafal-pracht
Copy link
Contributor Author

@mtreinish I've added those 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.

LGTM, thanks for keeping with this and making the updates

@mtreinish mtreinish changed the title Merge u1, u2 and p gate together if they contain parameters. Merge paramterized u1, u2 and p gate in Optimize1qGates Jan 28, 2022
@mtreinish mtreinish added the Changelog: New Feature Include in the "Added" section of the changelog label Jan 28, 2022
@mergify mergify bot merged commit 37f2c01 into Qiskit:main Jan 28, 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.

3 participants