[unitaryHACK] Add the CPhase operation

[nahumsa]

Context: Add a new diagonal operator, CPhase.

Description of the Change: Adds the CPhase operator as an alias for the ControlledPhaseShift operator.

Benefits: Adds the CPhase diagonal operator.

Possible Drawbacks: It has the same structure as ControlledPhaseShift, maybe adds duplicated code that could be solved by using CPhase as an alias for the ControlledPhaseShift.

Related GitHub Issues: closes #1147

[josh146]

Thanks @nahumsa! Let us know as soon as the PR is ready for review, and someone from the team will be on hand to answer any questions/provide a review 🙂

[nahumsa]

Hi @josh146. I guess this is ready for a review.

[nahumsa]

Could someone clarify why the tests/tape/test_reversible.py::TestGradients::test_gradients[CPhase-PauliX] would fail?
It was ok locally 😅

[nahumsa]

Is it ok just to remove qml.CPhase from analytic_qubit_ops, just like the qml.ControlledPhaseShift?

[codecov]

Codecov Report

Merging #1319 (97afc35) into master (b8ea0de) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1319   +/-   ##
=======================================
  Coverage   98.16%   98.16%           
=======================================
  Files         148      148           
  Lines       11456    11457    +1     
=======================================
+ Hits        11246    11247    +1     
  Misses        210      210           

Impacted Files	Coverage Δ
pennylane/devices/default_qubit.py	100.00% <ø> (ø)
pennylane/devices/default_qubit_autograd.py	100.00% <ø> (ø)
pennylane/devices/default_qubit_jax.py	94.36% <ø> (ø)
pennylane/devices/default_qubit_tf.py	90.66% <ø> (ø)
pennylane/ops/qubit.py	98.51% <100.00%> (+<0.01%)	⬆️

---

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 b8ea0de...97afc35. Read the comment docs.

[antalszava]

Hi @nahumsa, thank you for exploring this addition and identifying that we already have an equivalent gate!

Could we create the alias by having

CPhase = ControlledPhaseShift

in the pennylane/pennylane/ops/qubit.py file? That would essentially mean that CPhase is another name to the same class object as ControlledPhaseShift and could help with keeping the aliasing short.

[nahumsa]

Sure! I thought about doing just that, but at the end I think I overcomplicated the solution. 😅
Sorry about the overcomplication

[antalszava]

No worries at all! :) Let us know if you'd like us to go for another review round or if any questions come up.

[nahumsa]

Would it be good to remove the tests as well, or should I keep it? Since it is exactly the same tests as ControlledPhaseShift

[antalszava]

The test cases could be removed and potentially the existing ones that test the eigvals and decomposition of qml.ControlledPhaseShift could be parametrized such that:

    @pytest.mark.parametrize("phi", [-0.1, 0.2, 0.5])
    @pytest.mark.parametrize("cphase_op", [qml.ControlledPhaseShift, qml.CPhase])
    def test_controlled_phase_shift_decomp(self, phi):
        op = cphase_op(phi, wires=[0, 1])
        (...)

This will make sure that aliasing works fine.

[nahumsa]

The test cases could be removed and potentially the existing ones that test the eigvals and decomposition of qml.ControlledPhaseShift could be parametrized such that:

    @pytest.mark.parametrize("phi", [-0.1, 0.2, 0.5])
    @pytest.mark.parametrize("cphase_op", [qml.ControlledPhaseShift, qml.CPhase])
    def test_controlled_phase_shift_decomp(self, phi):
        op = cphase_op(phi, wires=[0, 1])
        (...)

This will make sure that aliasing works fine.

Awesome, just added this parameterized test, thanks!

[antalszava]

Great! 🙏 The aliasing could be used in framework-specific files too, so we could assign the CPhase name the underlying ControlledPhaseShift function in each file to have the same effect. :)

[nahumsa]

Great! The aliasing could be used in framework-specific files too, so we could assign the CPhase name the underlying ControlledPhaseShift function in each file to have the same effect. :)

Sure. I'll do that.

[antalszava]

Awesome! :) This is looking great, left two comments, but very near being ready. 🙂

[antalszava]

Looks good to me! 🎉 🙂 Thank you @nahumsa for your contribution and for tracking down how exactly it's best to have this addition! 💪

[nahumsa]

Looks good to me! Thank you @nahumsa for your contribution and for tracking down how exactly it's best to have this addition!

Thank you, @antalszava, for such an amazing guidance through this PR! 😄