-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Decomposition of single R gate in RR basis should be one R gate, not two #11304
Conversation
Thank you for opening a new pull request. Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient. While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone. One or more of the the following people are requested to review this:
|
Currently, a single R gate may come out of RR-decomposition as two R gates, which is obviously not ideal. For example, `RGate(0.1, 0.2)` is currently decomposed (in RR basis) as ``` ┌────────────────┐┌──────────┐ 0: ┤ R(-3.0416,0.2) ├┤ R(π,0.2) ├ └────────────────┘└──────────┘ ``` Two `R(𝜗, 𝜑)` gates with the same 𝜑 parameter can be combined into one by simply adding up the 𝜗 values (giving us `R(0.1, 0.2)`, unsurprisingly). In terms of `U(𝜗, 𝜑, 𝜆)`, it is the case when `𝜑 = -𝜆` that the two R-gates we construct have the same second parameter and therefore should be expressed as a single R gate. For example, `U3Gate(0.1, 0.2, -0.2)` currently produces this RR-decomposition: ``` ┌───────────────────┐┌─────────────┐ 0: ┤ R(-3.0416,1.7708) ├┤ R(π,1.7708) ├ └───────────────────┘└─────────────┘ ``` which also unnecessarily consists of two R gates instead of just one. This commit adds the two examples above as unit tests, ensuring they RR-decompose two just one R gate, as well as the code changes to make these two new tests pass, along with all existing tests, of course. The condition for this special case is that the 𝜑 parameters of the two R-gates we would emit are the same (thus `mod_2pi(PI / 2. - lam)=mod_2pi(0.5 * (phi - lam + PI)`, simplified as `mod_2pi((phi + lam) / 2)=0`).
fe3da85
to
ee2fa15
Compare
Pull Request Test Coverage Report for Build 7030458574Warning: This coverage report may be inaccurate.We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this! Sorry for the slightly delayed response. It's great to get these inefficiencies ironed out in the less-used decomposition targets.
Would you mind adding a feature release note mentioning the improved synthesis quality (reno new --edit rr-decomposition-synthesis
or similar)? I can do it if it's easier, though.
No problem, @jakelishman! I had a go. Feel free to edit if it doesn't quite sit right. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks fine to me, thanks for finding it, and for contributing the fix!
Summary
Currently, a single R gate may come out of RR-decomposition as two R gates, which is obviously not ideal.
This PR adds test cases for when this happens and contains the code changes to only emit one R gate when possible.
Details and comments
For example,
RGate(0.1, 0.2)
is currently decomposed (in RR basis) asTwo
R(𝜗, 𝜑)
gates with the same 𝜑 parameter can be combined into one by simply adding up the 𝜗 values (giving usR(0.1, 0.2)
, unsurprisingly).In terms of
U(𝜗, 𝜑, 𝜆)
, it is the case when𝜑 = -𝜆
that the two R-gates we construct have the same second parameter and therefore should be expressed as a single R gate. For example,U3Gate(0.1, 0.2, -0.2)
currently produces this RR-decomposition:which also unnecessarily consists of two R gates instead of just one.
This commit adds the two examples above as unit tests, ensuring they RR-decompose two just one R gate, as well as the code changes to make these two new tests pass, along with all existing tests, of course.
The condition for this special case is that the 𝜑 parameters of the two R-gates we would emit are the same (thus
mod_2pi(PI / 2. - lam)=mod_2pi(0.5 * (phi - lam + PI)
, simplified asmod_2pi((phi + lam) / 2)=0
).