-
Notifications
You must be signed in to change notification settings - Fork 657
[Decomposition] Custom decomposition rules for symbolic operators #7347
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
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7347 +/- ##
==========================================
- Coverage 99.65% 99.65% -0.01%
==========================================
Files 529 529
Lines 50603 50600 -3
==========================================
- Hits 50427 50424 -3
Misses 176 176 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…o symbolic-rules-01
…o symbolic-rules-01
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'll complete the review as soon as possible (need to interrupt for a few minutes :) )
Co-authored-by: Pietropaolo Frisoni <[email protected]>
…o symbolic-rules-01
…o symbolic-rules-01
…o symbolic-rules-01
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 don't have more questions here! Just a good-to-have non-blocking comment
if issubclass(op.op_type, qml.ops.Adjoint): | ||
decomps.extend(self._get_adjoint_decompositions(op)) | ||
|
||
elif issubclass(op.op_type, qml.ops.Pow): | ||
decomps.extend(self._get_pow_decompositions(op)) | ||
|
||
elif op.op_type in (qml.ops.Controlled, qml.ops.ControlledOp): |
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.
We do we use issubclass
for adjoint and pow but op_type in
for controlled? Seems like it might be simpler to use the same pattern for all three categories.
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.
Note that AdjointOpObs
and PowOpObs
will be going away in July, so we can potentially make decisions based around it being removed.
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.
For Controlled
, we don't want any of the custom controlled operations like CH
, CNOT
, etc. to take this branch, but for Adjoint
, we want all Adjoint
ops including AdjointOperation
to take this branch.
@@ -208,88 +215,65 @@ def _add_decomp_rule_to_op( | |||
except DecompositionNotApplicable: | |||
pass # ignore decompositions that are not applicable to the given op params. | |||
|
|||
def _add_adjoint_decomp_node(self, op_node: CompressedResourceOp, op_node_idx: int) -> int: | |||
"""Adds an adjoint decomposition node.""" | |||
def _get_adjoint_decompositions(self, op: CompressedResourceOp) -> list[DecompositionRule]: |
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.
What about using something like resource_op
when we are dealing with a CompressedResourceOp
. Might make it a little clearer what of object we are dealing with, as I tend to associate op
with Operator
.
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.
Sure
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'm renaming them back to op_node
for consistency in the context of the graph.
…o symbolic-rules-01
Context:
Description of the Change:
Symbolic operator types can now be given as strings to the
op_type
argument ofadd_decomps
or as keys of the dictionaries passed to thealt_decomps
andfixed_decomps
arguments of thedecompose
transform, allowing custom decomposition rules to be defined and registered for symbolic operators.>>> print(qml.draw(circuit)()) 0: ──RX(-0.50)─╭●────────────┤ <Z> 1: ────────────╰X──RY(-0.50)─┤
Benefits:
Possible Drawbacks:
Related GitHub Issues:
[sc-89297]