Deprecate QubitUnitary as a valid input to ControlledQubitUnitary
#6840
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hello. You may have forgotten to update the changelog!
|
JerryChen97
added
the
deprecation 🗑️
JerryChen97
changed the base branch from
master
to
Deprecate-`control_wires`-in-`ControlledQubitUnitary`-in-favour-of-`wires`
…n-favour-of-`wires`' into Deprecate-`QubitUnitary`-as-a-valid-input-to-`ControlledQubitUnitary`
This comment was marked as outdated.
This comment was marked as outdated.
|
@albi3ro @astralcai Here's how I deprecate the |
astralcai
reviewed
Jan 24, 2025
JerryChen97
commented
Jan 24, 2025
JerryChen97
commented
Jan 24, 2025
astralcai
reviewed
Jan 28, 2025
albi3ro
reviewed
Jan 28, 2025
astralcai
reviewed
Jan 28, 2025
There was a problem hiding this comment.
I just realized something: for every other controlled operator, its base property is an operator. For example:
>>> op = qml.CRX(0.5, [0, 1])
>>> op.base
RX(0.5, wires=[1])Should we maintain the same behaviour for ControlledQubitUnitary? @albi3ro
EDIT:
To be more consistent with how every other custom controlled operation works, the ControlledQubitUnitary should probably create a QubitUnitary internally and set its base to that.
EDIT:
nvm I'm dumb.
Co-authored-by: Astral Cai <[email protected]>
…tary`-as-a-valid-input-to-`ControlledQubitUnitary`' into Deprecate-`QubitUnitary`-as-a-valid-input-to-`ControlledQubitUnitary`
…n-favour-of-`wires`' into Deprecate-`QubitUnitary`-as-a-valid-input-to-`ControlledQubitUnitary`
albi3ro
reviewed
Jan 28, 2025
Co-authored-by: Christina Lee <[email protected]>
…n-favour-of-`wires`' into Deprecate-`QubitUnitary`-as-a-valid-input-to-`ControlledQubitUnitary`
…n-favour-of-`wires`' into Deprecate-`QubitUnitary`-as-a-valid-input-to-`ControlledQubitUnitary`
albi3ro
reviewed
Jan 29, 2025
Co-authored-by: Christina Lee <[email protected]>
astralcai
approved these changes
Jan 29, 2025
albi3ro
approved these changes
Jan 29, 2025
JerryChen97
merged commit 51fb48c
into
Deprecate-`control_wires`-in-`ControlledQubitUnitary`-in-favour-of-`wires`
42 of 47 checks passed
JerryChen97
deleted the
Deprecate-`QubitUnitary`-as-a-valid-input-to-`ControlledQubitUnitary`
branch
JerryChen97
added a commit
to PennyLaneAI/pennylane-lightning
that referenced
this pull request
Feb 3, 2025
**Context:** Due to the deprecations of `ControlledQubitUnitary` input args PennyLaneAI/pennylane#6839 PennyLaneAI/pennylane#6840, we need to update several tests of lightning that use deprecated interfaces. **Description of the Change:** **Benefits:** **Possible Drawbacks:** **Related GitHub Issues:** [sc-80842] --------- Co-authored-by: ringo-but-quantum <[email protected]> Co-authored-by: Alex Preciado <[email protected]>
JerryChen97
added a commit
that referenced
this pull request
Feb 6, 2025
**Context:** A recent deprecation #6840 deprecated the usage of `QubitUnitary` type input `base` for instantiation of `ControlledQubitUnitary`. However, a modification that was equivalent in PennyLane caused unexpected behavior in Catalyst, PennyLaneAI/catalyst#1494. After investigation, it appears to be the issue with the private method `_try_wrap_in_custom_ctrl_op` of `pennylane/ops/op_math/controlled.py`, since it used to call the deprecated `init` to directly use base as `QubitUnitary`, and to safely deprecate it we decided to locally replace with equivalent `ControlledQubitUnitary(base=op.matrix(), ...)`, this caused Catalyst to received double `QubitUnitary`. It is also noteable that the call `ControlledQubitUnitary` should not appear in this script, since logically all the classes living in `controlled_ops` depend on `controlled.py`, which means that from the very beginning the affected private method served as a tricky hack with cyclic dependence. Therefore, we would like to replace the `ControlledQubitUnitary` call with a direct `ControlledOp`. **Description of the Change:** source files: as mentioned above. tests: necessary improvements were made to correctly reflect the assertion results. **Benefits:** - Remove bugs - Less cyclic dependency **Possible Drawbacks:** - Probably there're more that depend on this method, which might be affected by this fix **Related GitHub Issues:** PennyLaneAI/catalyst#1494 --------- Co-authored-by: Christina Lee <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context:
Providing a
QubitUnitaryas the base operator defeats the purpose entirely and is just a messier way of usingqml.ctrl(qml.QubitUnitary(...), ...)For richer context, check #6839 for better illustration; this PR should in principle be an add-on to it.
Note: this deprecation is related with a previous bug. Trace via attached story link to see full discussions.
Description of the Change:
__init__and_unflatten: If get aQubitUnitarybase, raise PLDW and then get matrix bybase.matrix().Added an overloading method
_flatten. This is a fix to #6839.Deleted two tests that restrictively replied on the support of
QubitUnitarybase.Benefits:
Possible Drawbacks:
Related GitHub Issues:
[sc-80843]