[Code Together] Adding SISWAP operation

[charmerDark]

Hey @antalszava
I've just added the SISWAP class, but I am having trouble exposing it as qml.SISWAP . I tried following the PR for ISWAP. But I think the code structure has changed a bit since then? Could you please help me out on this end?

Related issues

Closes #1527

[antalszava]

Hi @charmerDark, after checking out your branch I could access qml.SISWAP 🤔 Was there a particular place in the codebase where it was not working?

The operations have indeed been split across files. If a new op is added to one of the existing files, it should be imported by in the top level __init__.py file.

[codecov]

Codecov Report

Merging #1563 (a4120f1) into master (566e104) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1563   +/-   ##
=======================================
  Coverage   99.16%   99.16%           
=======================================
  Files         189      189           
  Lines       13625    13642   +17     
=======================================
+ Hits        13511    13528   +17     
  Misses        114      114           

Impacted Files	Coverage Δ
pennylane/devices/default_qubit.py	100.00% <ø> (ø)
pennylane/ops/qubit/__init__.py	100.00% <ø> (ø)
pennylane/ops/qubit/non_parametric_ops.py	96.82% <100.00%> (+0.13%)	⬆️

---

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 566e104...a4120f1. Read the comment docs.

[charmerDark]

Hey @antalszava !
I hadn't added any tests for SQISW because it redirects to SISWAP class. Also should I mention SQISW in the documentation line for SISWAP? Please let me know any other changes I should make
The earlier issue with the SISWAP class was because of my local python setup I guess . It works well everywhere else.

[antalszava]

Hi @charmerDark, just for extra safety, cases could be added for qml.SQISW too, similar to how it's the case for CPhase:

• pennylane/tests/ops/test_qubit_ops.py

  Line 431 in e44a327

  qml.CPhase(1.777, wires=[0, 2]),

• pennylane/tests/ops/test_qubit_ops.py

  Line 1556 in e44a327

  @pytest.mark.parametrize("cphase_op", [qml.ControlledPhaseShift, qml.CPhase])

It's mostly a matter of adding the same test case as for qml.SISWAP, just calling qml.SQISW.

Also should I mention SQISW in the documentation line for SISWAP?

Yep, it could also be added to doc/introduction/operations.rst, just is the case with CPhase.

[charmerDark]

Hey @antalszava
Could you see if its all right now? Thanks in advance

[antalszava]

Hi @charmerDark, this is getting there! 🎉 Left some suggestions, but the main addition is looking nice. 🙂

[antalszava]

Hi @charmerDark, this is looking great! 🎉 Thank you for the addition and for following through the review process. 😊 🥇