PennyLaneAI / pennylane-aqt

The PennyLane-AQT plugin integrates Alpine Quantum Technologies' ion-trap quantum computing hardware with with PennyLane's quantum machine learning capabilities.
https://docs.pennylane.ai/projects/aqt
Apache License 2.0
9 stars 3 forks source link

CNOT gate doesn't work with aqt.sim #24

Closed muttley2k closed 2 years ago

muttley2k commented 2 years ago

Hi there. The following code should return 1.0. It does so with default.qubit, but with aqt.sim it returns a result close to 0.0:

import pennylane as qml

#dev = qml.device("default.qubit", wires=2)
dev = qml.device("aqt.sim", wires=2, api_key="<Insert API Key here>")

@qml.qnode(dev)
def circuit():
    qml.CNOT(wires=[0, 1])
    qml.CNOT(wires=[0, 1])
    return qml.expval(qml.PauliZ(0) @ qml.PauliZ(1))

print(circuit())

drawer = qml.draw(circuit)
print(drawer())

Output:

0.11
 0: ──╭C──╭C──╭┤ ⟨Z ⊗ Z⟩ 
 1: ──╰X──╰X──╰┤ ⟨Z ⊗ Z⟩ 
muttley2k commented 2 years ago

I think I found what the issue is.

In the paper where the formula was taken from in #14, the angle is confusing:

image

In the picture above, XX(theta) means MS(2*theta), so I think the fix would be to use np.pi / 2 in the decomposition, not np.pi / 4:

        if op_name == "CNOT":
            self._append_op_to_queue("RY", np.pi / 2, [device_wire_labels[0]])
            #self._append_op_to_queue("MS", np.pi / 4, device_wire_labels)
            self._append_op_to_queue("MS", np.pi / 2, device_wire_labels)
            self._append_op_to_queue("RX", -np.pi / 2, [device_wire_labels[0]])
            self._append_op_to_queue("RX", -np.pi / 2, [device_wire_labels[1]])
            self._append_op_to_queue("RY", -np.pi / 2, [device_wire_labels[0]])
            return
antalszava commented 2 years ago

Hi @muttley2k,

Thank you for reporting this and for thoroughly looking into a fix here! :slightly_smiling_face: Just to confirm, are results as expected after the change?

Let us know if you'd like to open a pull request with the changes.

muttley2k commented 2 years ago

Hi @muttley2k,

Thank you for reporting this and for thoroughly looking into a fix here! 🙂 Just to confirm, are results as expected after the change?

Let us know if you'd like to open a pull request with the changes.

@antalszava Yes, locally the results look good to me, now the IsingXX gate as well as UCCSD works as expected, using aqt.sim. It could be a 1-character change, unless unit tests are obligatory (they didn't help before it seems).

muttley2k commented 2 years ago

I can do the pull request, it will be a 2-character change if the unit test is adjusted too.

CatalinaAlbornoz commented 2 years ago

Thanks @muttley2k ! Feel free to do the pull request and link this issue there too.