Fix ControlledPhaseShift

[ryanlevy]

Context:

In PR #1064 the new ControlledPhaseShift was added, however the CNOT wires are fixed to 0 and 1 instead of the user input

Description of the Change: Wires in the CNOT decomposition are no longer fixed to 0 and 1
Benefits: Fixes new operation
Possible Drawbacks: None
Related GitHub Issues: None?

[josh146]

Wow! Great catch @ryanlevy. Thanks for this!

This is something that definitely should have been picked up by our tests, so a sign that our tests are lacking 🙂 Do you think you will be able to add a test that ensures the decomposition is correct for a weird set of wires (e.g., wires=[2, 0])?

It could be something as simple as

original_mat = qml.ControlledPhaseShift(x, wires=[2, 0]).matrix
decomposed_mats = [op.matrix for op in qml.ControlledPhaseShift.decomposition(x, wires=[2, 0])]

and performing matrix multiplication to ensure that the original matrix and decomposed matrix match.

[josh146]

By the way, don't forget to update the changelog with a link back to this PR, and add your name to the contributors section :)

[codecov]

Codecov Report

Merging #1338 (fb03b09) into master (24e9360) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1338   +/-   ##
=======================================
  Coverage   98.16%   98.16%           
=======================================
  Files         145      145           
  Lines       11093    11093           
=======================================
  Hits        10889    10889           
  Misses        204      204           

Impacted Files	Coverage Δ
pennylane/ops/qubit.py	98.53% <ø> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 24e9360...fb03b09. Read the comment docs.

[ryanlevy]

Thanks for the reminder!

I've updated the tests, but the .matrix is a bit lacking to really test things (CNOT's matrix is the same if wires=[0,1] or wires=[1,0] for example). Hope this is ok!

(see #1313)

[josh146]

CNOT's matrix is the same if wires=[0,1] or wires=[1,0] for example

Ah, of course! This is because PennyLane allows arbitrary labels for wires (e.g., CNOT(wires=['a', 0])), and as a consequence of this, wire labels have no implicit ordering.

The only time wire ordering can be deduced is if the operation is provided alongside a device --- the ordering of the wires on the device is considered canonical.

[josh146]

Looks great from my end, thanks @ryanlevy!