Use pass_name from pennylane transform
#2149
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2149 +/- ##
=======================================
Coverage 97.49% 97.50%
=======================================
Files 93 93
Lines 10886 10921 +35
Branches 1038 1046 +8
=======================================
+ Hits 10613 10648 +35
Misses 211 211
Partials 62 62 ☔ 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
mudit2812
left a comment
There was a problem hiding this comment.
This looks great to me! I'll let the compilation team be the final approvers here
| def _lowered_options(args, kwargs): | ||
| lowered_options = {} | ||
| for arg in args: | ||
| lowered_options[str(arg)] = get_mlir_attribute_from_pyval(True) | ||
| for option, value in kwargs.items(): | ||
| mlir_option = str(option).replace("_", "-") | ||
| lowered_options[mlir_option] = get_mlir_attribute_from_pyval(value) | ||
| return lowered_options |
There was a problem hiding this comment.
I would like this functionality to be extended in the future to also allow a way to lower python objects that do not have a straightforward way of lowering to MLIR. This is necessary for some of the xDSL passes to work. It would be very cumbersome for the default pass decorators to use arguments that are MLIR compatible for complex passes.
There was a problem hiding this comment.
But this has to be per-pass right? We don't know what python objects a xdsl pass might take in as options in general.
There was a problem hiding this comment.
Yep, passes would need to define their own strategies for serializing/deserializing, but the compilation pipeline will have to be updated to integrate with these strategies.
There was a problem hiding this comment.
But I don't know what concrete actions we can do now without knowing what the specific passes need 🤔
There was a problem hiding this comment.
Put something in a backlog TODO for now?
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]