-
Notifications
You must be signed in to change notification settings - Fork 35
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 merge rotation pass #1162
Add merge rotation pass #1162
Conversation
Note that I added an analysis object in #1186 to check for if a gate operation and its parent gate operation are correctly matched for the purposes of peephole optimizations like merge rotations and cancel inverses. It might be beneficial to merge that PR and use the added analysis if you want. |
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, looks good! Just some small points from me
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1162 +/- ##
==========================================
- Coverage 97.93% 97.90% -0.03%
==========================================
Files 77 77
Lines 10921 10935 +14
Branches 971 973 +2
==========================================
+ Hits 10695 10706 +11
- Misses 179 180 +1
- Partials 47 49 +2 ☔ View full report in Codecov by Sentry. |
I think codecov does not cover lit tests. One pytest in https://github.com/PennyLaneAI/catalyst/blob/main/frontend/test/pytest/test_peephole_optimizations.py |
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.
🥳
[sc-72014] |
**Context:** In #1162 the mlir pass for merging rotation was added, and the pattern included `qml.Rot` (and `CRot`). However, it was then realized that these two kinds of rotations should not be merged, as [the rotation is specified via ](https://docs.pennylane.ai/en/stable/code/api/pennylane.Rot.html) `rot(a, b, c) = rz(c) ry(b) rz(a)` This means `rot(a, b, c) rot(d, e, f) != rot(a+d, b+e, c+f)` since y and z rotations do not commute. **Description of the Change:** Remove `qml.Rot` (and `CRot`) from merge rotation pass patterns **Benefits:** No numerical errors
Description of the Change:
Add the merge rotations pass, rotations angles are summed in rotation gates removed. It works for multirzop, and customop
({"RX", "RY", "RZ", "PhaseShift", "Rot", "CRX", "CRY", "CRZ", "ControlledPhaseShift", "CRot"})
.