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

Fix controlled qubit unitary usage #1310

Merged
merged 9 commits into from
Feb 3, 2025
Merged

Conversation

JerryChen97
Copy link
Contributor

@JerryChen97 JerryChen97 commented Jan 23, 2025

Title:
Adjust interface of ControlledQubitUnitary to be compatible with PennyLane v0.41

Summary:
changed 5 occurences in demonstrations/tutorial_classically_boosted_vqe.py and demonstrations/tutorial_testing_symmetry.py

Related Shortcut Stories:
[sc-80842]

@JerryChen97 JerryChen97 self-assigned this Jan 23, 2025

This comment was marked as resolved.

@JerryChen97
Copy link
Contributor Author

Do not merge until PL updated

@JerryChen97
Copy link
Contributor Author

The PR to deprecate control_wires of ControlledQubitUnitary is very close to conclude, may I ask some pre-review for this? @isaacdevlugt @DSGuala

@andrijapau andrijapau self-requested a review January 31, 2025 15:07
Copy link
Contributor

@andrijapau andrijapau left a comment

Choose a reason for hiding this comment

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

Looks good!

@JerryChen97 JerryChen97 marked this pull request as ready for review January 31, 2025 20:02
JerryChen97 added a commit to PennyLaneAI/pennylane that referenced this pull request Jan 31, 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:
```python
mat = np.eye(2)
control_wires = [0]
wires = [1]
ControlledQubitUnitary(mat, control_wires, wires=wires, control_values=...)
```

After:
```python
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**
This is an inevitable breaking change, so we need to immediately inform
the affected repos that depend on latest PennyLane but only apply
corresponding changes after this PR gets merged.

- [ ] labs

- [ ] Catalyst:
[frontend/test/pytest/device/test_decomposition.py](https://github.com/PennyLaneAI/catalyst/blob/286e51910a39c55130de8c1c6a806818a2bcb14b/frontend/test/pytest/device/test_decomposition.py#L52)
PennyLaneAI/catalyst#1483

- [ ] Lightning: PR to fix it
PennyLaneAI/pennylane-lightning#1047
- -
[test_gates.py](https://github.com/PennyLaneAI/pennylane-lightning/blob/290c986e7d087e17dbf3f09c6d2dfc8bd738cc08/tests/test_gates.py#L451)
- -
[test_gates_and_expval.py](https://github.com/PennyLaneAI/pennylane-lightning/blob/290c986e7d087e17dbf3f09c6d2dfc8bd738cc08/tests/lightning_tensor/test_gates_and_expval.py#L72)
- -
[test_measurements_class.py](https://github.com/PennyLaneAI/pennylane-lightning/blob/290c986e7d087e17dbf3f09c6d2dfc8bd738cc08/tests/lightning_qubit/test_measurements_class.py#L814)

- [ ] QML Demos:
[tutorial_classically_boosted_vqe.py](https://github.com/PennyLaneAI/qml/blob/c444cec6201338056304d9d5a293f969a3aeec5e/demonstrations/tutorial_classically_boosted_vqe.py#L451);
PR to fix it PennyLaneAI/qml#1310

### **Plugins**
- [x] Pennylane-AQT: No instances of deprecated code found.
- [x] Pennylane-Qiskit: No instances of deprecated code found.
- [x] Pennylane-IonQ: No instances of deprecated code found.
- [x] Pennylane-Qrack: No instances of deprecated code found.
- [x] Pennylane-Cirq: No instances of deprecated code found.
- [x] 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]

---------

Co-authored-by: Christina Lee <[email protected]>
Co-authored-by: Astral Cai <[email protected]>
@JerryChen97 JerryChen97 enabled auto-merge (squash) February 1, 2025 00:04
@JerryChen97 JerryChen97 added ci:build-all-qml-demos Build all the Demos for this Pull Request and removed do not merge labels Feb 1, 2025
JerryChen97 and others added 3 commits January 31, 2025 19:05
@Alex-Preciado Alex-Preciado changed the base branch from master to dev February 2, 2025 00:58
@JerryChen97
Copy link
Contributor Author

JerryChen97 commented Feb 3, 2025

@Alex-Preciado The jax tutorial (not involved in this PR) in dev won't pass CI because of another known bug that is currently being fixed here: PennyLaneAI/pennylane#6899; related discusion can be found on slack

@Alex-Preciado Alex-Preciado changed the base branch from dev to master February 3, 2025 16:28
@JerryChen97
Copy link
Contributor Author

From PLcore standup 3 Feb 2025, we still need to convert base-branch back to dev branch. Merge this after the relevant bug is fixed.

@JerryChen97 JerryChen97 changed the base branch from master to dev February 3, 2025 16:49
@JerryChen97
Copy link
Contributor Author

On the modifications I made in this PR, do we have any concerns from product? @isaacdevlugt For example, I'm not sure if writing it like list(...) is too ugly to show our users😅

Copy link
Contributor

@isaacdevlugt isaacdevlugt left a comment

Choose a reason for hiding this comment

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

Thanks @JerryChen97! Works well for me :)

@JerryChen97 JerryChen97 enabled auto-merge (squash) February 3, 2025 21:19
@JerryChen97 JerryChen97 merged commit 68c6f08 into dev Feb 3, 2025
19 checks passed
@JerryChen97 JerryChen97 deleted the fix-ControlledQubitUnitary-usage branch February 3, 2025 21:58
Copy link

github-actions bot commented Feb 3, 2025

Thank you for opening this pull request.

You can find the built site at this link.

Deployment Info:

  • Pull Request ID: 1310
  • Deployment SHA: 8fb83b04686dd3d64bce576160dfc510f2940d56
    (The Deployment SHA refers to the latest commit hash the docs were built from)

Note: It may take several minutes for updates to this pull request to be reflected on the deployed site.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:build-all-qml-demos Build all the Demos for this Pull Request do not merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants