Skip to content
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

Deprecate control_wires in ControlledQubitUnitary #6839

Open
wants to merge 36 commits into
base: master
Choose a base branch
from

Conversation

JerryChen97
Copy link
Contributor

@JerryChen97 JerryChen97 commented Jan 15, 2025

Context:
We are in favor of wires, just like what we did with MultiControlledX.

!Note: this involves a big user-interface change. With PL version <=0.40, for ControlledQubitUnitary, we have arg wires to be the target_wires, and the arg control_wires is a positional arg. To be consistent with all the other interfaces, we would like to alter this into wires to be positional and to represent control_wires+target_wires

Before:

mat = np.eye(2)
control_wires = [0]
wires = [1]
ControlledQubitUnitary(mat, control_wires, wires=wires, control_values=...)

After:

mat = np.eye(2)
wires = [0, 1]
ControlledQubitUnitary(mat, wires, control_values=...)

where the new wires should be the concatenate of old control_wires and wires

Description of the Change:
Besides the modified test files across the whole codebase involving ControlledQubitUnitary, we made the following necessary adjustments beyond the __init__ method:

  • classmethod _unflatten, _primitive_bind_call and method _controlled were adjusted accordingly following the new interface
  • move the TypeError regarding empty arg wires to the beginning.
  • add a register for ControlledQubitUnitary in bind_new_parameters.py following the pattern of CRX
  • modified the direct calls of ControlledQubitUnitary inside src files metric_tensor.py, controlled.py and matrix_ops.py.

Eco-System

Plugins

  • Pennylane-AQT: No instances of deprecated code found.
  • Pennylane-Qiskit: No instances of deprecated code found.
  • Pennylane-IonQ: No instances of deprecated code found.
  • Pennylane-Qrack: No instances of deprecated code found.
  • Pennylane-Cirq: No instances of deprecated code found.
  • Pennylane-Qulacs: No instances of deprecated code found.

Benefits:
Unified interfaces with other controlled ops.

Possible Drawbacks:
This is a hard change to deprecate this arg. Would have to introduce errors for testing matrices.

Related GitHub Issues:
[sc-80842]

Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit doc/releases/changelog-dev.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@JerryChen97 JerryChen97 self-assigned this Jan 15, 2025
@JerryChen97 JerryChen97 changed the title Deprecate control_wires in ControlledQubitUnitary in favour of `w… Deprecate control_wires in ControlledQubitUnitary Jan 15, 2025
@JerryChen97 JerryChen97 added the deprecation 🗑️ Deprecating an API or user-facing feature/behaviour label Jan 15, 2025
@JerryChen97 JerryChen97 marked this pull request as ready for review January 16, 2025 22:39
Copy link

codecov bot commented Jan 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.49%. Comparing base (666941f) to head (980135f).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6839      +/-   ##
==========================================
- Coverage   99.54%   99.49%   -0.06%     
==========================================
  Files         477      477              
  Lines       45246    45331      +85     
==========================================
+ Hits        45041    45102      +61     
- Misses        205      229      +24     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Alex-Preciado Alex-Preciado requested review from albi3ro and removed request for lillian542 January 20, 2025 16:59

if hasattr(base, "wires") and len(wires) != 0:
warnings.warn(
"base operator already has wires; values specified through wires kwarg will be ignored."
)
wires = Wires(())
# base should have the target_wires. Then control_wires should be the ones in wires that are not in base.wires
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel that specifying wires with base being an operator is going to be ambiguous regardless of what we do. I believe we're deprecating that soon as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. Since this PR is altering way more than the other I did this first. Anything comes here is temprory and handwaving just to pass existing usage.
My plan for deprecating Operator input is to take the matrix and wires out from the op along with raising warning, and then fall back to normal process.

…wires`-in-`ControlledQubitUnitary`-in-favour-of-`wires`' into Deprecate-`control_wires`-in-`ControlledQubitUnitary`-in-favour-of-`wires`
JerryChen97 and others added 2 commits January 24, 2025 12:15
Co-authored-by: Astral Cai <astral.cai@xanadu.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:run-full-test-suite Run the full test-suite on the pull request deprecation 🗑️ Deprecating an API or user-facing feature/behaviour
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants