Fix decomposition for inverse S and T gates

[josh146]

Context:

Two bugs were recently discovered in #955.

• The tape QNode was decomposing gates based on the contents of device.operations. Instead, it should have been using device.supports_operation(), which has additional logic to deal with inverses.

• The new Operation.expand() method which is used in tape mode was not inverting the decomposed tape if the original op was inverted.

Description of the Change:

• The tape QNode expand logic has been changed to use device.supports_operation().

• The Operation.expand() method now inverts the expanded tape if the operation was inverted.

• Adds tests to make sure this logic works.

Benefits:

• Devices that don't natively support qml.S.inv() and qml.T.inv() are now provided with the correct decomposition.

• In tape mode, devices that do support qml.S.inv() and qml.T.inv() do not have these operations decomposed.

Possible Drawbacks: n/a

Related GitHub Issues: Fixes #995

[codecov]

Codecov Report

Merging #956 (29d6be4) into master (967ed1d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #956   +/-   ##
=======================================
  Coverage   97.82%   97.82%           
=======================================
  Files         149      149           
  Lines       10236    10237    +1     
=======================================
+ Hits        10013    10014    +1     
  Misses        223      223           

Impacted Files	Coverage Δ
pennylane/tape/operation.py	98.41% <100.00%> (+0.05%)	⬆️
pennylane/tape/qnode.py	98.14% <100.00%> (-0.02%)	⬇️

---

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 967ed1d...29d6be4. Read the comment docs.

[glassnotes]

Thanks for taking care of this so quickly @josh146 !

[antalszava]

Hi @josh146, good catch with the decomposition! I'm a bit still hesitant with non-tape mode, however.

Would the inverse of the decomposition not be accounted for in the QNode? 🤔

pennylane/pennylane/qnodes/base.py

Line 100 in c0d269e

decomposed_ops = op.decomposition(*op.data, wires=op.wires)

When decomposing the inverse of the S or T gates, by taking the inverse of the decomposition on the operation level, it seems that we end up taking the inverse twice overall (and simply applying S or T eventually):

import pennylane as qml

qml.disable_tape()
dev = qml.device('default.qubit.autograd', wires=1)

if "S" in dev.operations:
    dev.operations.remove("S")

@qml.qnode(dev)
def circuit():
    qml.Hadamard(wires=0)
    qml.S(wires=0).inv()
    return qml.expval(qml.PauliZ(0))

circuit()
dev.state

tensor([7.07106781e-01+0.j        , 4.32978028e-17+0.70710678j], requires_grad=True)

The same example of using the decomposition in tape mode:

tensor([7.07106781e-01+0.j        , 4.32978028e-17-0.70710678j], requires_grad=True)

Perhaps adding such examples (or similar) as higher-level integration tests could help here.

[josh146]

@antalszava you're entirely right, thank you for noticing that! Considering your comment, I've done the following:

• Removed the inverse branches in the S and T decompositions,

• Modified Operation.expand() so that if the original operation is inverted, so is the final decomposition,

• Added a high-level integration test to check all combinations of tape/non-tape mode, and decompositions/non-decompositions.

[antalszava]

Looks good to me! 🥇 Great fix 🙂 Was wondering about a test case for T too, probably the S test case sort of covers the specific case though (as T and S are very similar gates and the operations themselves are tested separately).

[antalszava]

💯