-
Notifications
You must be signed in to change notification settings - Fork 58
Use pass_name from pennylane transform
#2149
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2149 +/- ##
=======================================
Coverage 97.55% 97.56%
=======================================
Files 93 93
Lines 11184 11219 +35
Branches 1065 1073 +8
=======================================
+ Hits 10911 10946 +35
Misses 208 208
Partials 65 65 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Don't forget changelog (we can probably just add this one to the single transform primitive one) |
Co-authored-by: Paul <[email protected]>
Co-authored-by: Paul <[email protected]>
mudit2812
left a comment
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 great to me! I'll let the compilation team be the final approvers here
sengthai
left a comment
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.
LGTM 👍
Context:
Trying to improve our integration between pennylane transforms and catalyst passes.
See #2206 for where we are able to update all built-in catalyst passes to be transforms instead. That PR helps validate we can make this change with no breaking behaviour.
Going forward, we can setup a plan to get rid of the catalyst-specific pass handling infrastructure.
Also note that this PR targets the PennyLane branch, so we can confirm we won't need anymore changes there to get catalyst working correctly before approving and merging.
Description of the Change:
Uses the
transform.pass_nameproperty as the higher priority source of thepass_name.Updates the
pass_pipelinefrom thequantum_kernel_pprimitive to be a tuple ofTransformContainerobjects instead ofPass, but leaves in the logic to handletuple[Pass,...]for safer backwards compatibility.Also makes using transforms with pass names by splitting a
QNode's transform program into "things at the start of the program that are tape transforms" and "things at the end of the program that are MLIR passes". This will allow us to get rid ofPassPipelineWrapperuse for both systems and soften the change from old frontend to new frontend.Benefits:
Unified handling of transforms across pennylane and catalyst.
Possible Drawbacks:
Related GitHub Issues:
Depends on PennyLaneAI/pennylane#8539 [sc-103775]