-
Notifications
You must be signed in to change notification settings - Fork 156
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
Add plugin pass to support non-calibrated rzz angles #2043
base: main
Are you sure you want to change the base?
Add plugin pass to support non-calibrated rzz angles #2043
Conversation
Thanks @nkanazawa1989. As much as users will certainly be happy to have all angle values covered, it will also be confusing for them to see that values that are supposed to be invalid pass all the checks and run successfully. Therefore:
|
if angle != wrap_angle: | ||
dag.substitute_node( | ||
node, | ||
RZZGate(wrap_angle), | ||
inplace=True, | ||
) | ||
continue |
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.
Please explain, thanks
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.
I understand what confuses me. On the one hand, from what I see the range to consider should be [-2pi, 2pi] (see https://github.com/Qiskit/qiskit-ibm-runtime/pull/2043/files#r1841666155). On the other hand, I must get something wrong, because according to the code here the period is 2pi.
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.
After phase wrapping you always get [-pi, pi]
("quad1", 0.1), | ||
("quad2", pi / 2 + 0.1), | ||
("quad3", -pi + 0.1), | ||
("quad4", -0.1), |
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.
The rzz matrix seems to have a period of 4pi. Is it correct? If yes, then I believe the cases of -3pi/2 and 3pi should also be included.
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 is too much for ddt because it is identical to validating the logic of phase wrapping. Instead I designed another test to cover more angles in 0bc77b9
|
||
def run(self, dag: DAGCircuit) -> DAGCircuit: | ||
# Currently control flow ops and Rzz cannot live in the same circuit. | ||
# Once it's supported, we need to recursively check subroutines. |
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.
I prefer to already support subroutines, to avoid pitfalls when the time comes and dynamic circuits are allowed.
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.
I usually don't like writing unreachable logic because of the maintenance overhead. But the code is in 013a6b4
What I wrote here is not accurate. An ISA circuit is still a circuit where all rzz angles are in the range [0, pi/2]. How can we let the users know that, if they transpile with a backend that enables fractional gates, then they enjoy the service of the transpiler taking care of the angle values for them? |
Co-authored-by: Yael Ben-Haim <[email protected]>
Co-authored-by: Yael Ben-Haim <[email protected]>
Co-authored-by: Yael Ben-Haim <[email protected]>
The documentation that you added is very good. I think the general documentation of fractional gates should be updated somehow. Right now it says that theta must be between 0 and 2pi, and does not mention that you don't have to worry about it if you only transpile your circuit. |
+1. @kaelynj can you update the doc? |
Just not yet... Thanks 😄. |
Be aware of Qiskit/qiskit#13320. It's planned for Qiskit 2.0. #2043 should be merged first. |
Thanks for linking the issue. Probably it's safer to merge this first as you mention. Qiskit core doesn't need to consider angle restriction because this is very IBM Quantum specific. |
@kt474 I'm reviewing the transpiler pass, but can you please review the design of the entry point |
Summary
IBM Quantum recently released new basis set including rzz gate. The Rzz gate supports continuous angle but it is calibrated only within [0, pi/2]. We have implemented the validation mechanism to detect non-calibrated angles, but user always need to manually tweak circuits to avoid angles outside this range, which is a bit cumbersome.
This PR adds a plugin pass to automatically modifies the input circuit so that end users don't need to consider our hardware constraints. However, a user still needs to consider the constraints when they send parameterized PUB with Rzz gates. The latter case is outside the scope of this PR.
Details and comments
This PR adds new translation plugin. Alternatively we can add new pass to existing
ibm_dynamic_circuits
entry point, but name is very specific to the dynamic circuits feature and adding fractional feature under this plugin name seems confusing. For now I decided to create new entry pointibm_fractional
but eventually these must be merged when our hardware supports both features with a single execution path.