[BUG] Unexpected behaviour when using tape.to_openqasm()

trbromley commented 3 years ago

Expected behavior

Consider the tape

import pennylane as qml

with qml.tape.QuantumTape() as tape:
    qml.PauliX(wires=1)
    qml.PauliY(wires=0)

Note that the first gate is applied to wire 1 and tape.wires = <Wires = [1, 0]>.

We know convert to QASM:

>>> print(tape.to_openqasm())
OPENQASM 2.0;
include "qelib1.inc";
qreg q[2];
creg c[2];
x q[0];
y q[1];
measure q[0] -> c[0];
measure q[1] -> c[1];

We see that the wires have been permuted, i.e., with X being applied to wire 0 and Y being applied to wire 1.

This can be fixed by specifying a wire ordering in to_openqasm():

>>> print(tape.to_openqasm(wires=sorted(tape.wires)))
OPENQASM 2.0;
include "qelib1.inc";
qreg q[2];
creg c[2];
x q[1];
y q[0];
measure q[0] -> c[0];
measure q[1] -> c[1];

Actual behavior

The permuted wire ordering is unexpected behaviour and seems to arise from the tape wires not being ordered, i.e., tape.wires = <Wires = [1, 0]>. Another side-effect of non-ordered tape wires is that tape.draw() can potentially be surprising:

>>> tape.draw()
 1: ──X──┤  
 0: ──Y──┤

(again, fixable by specifying a wire ordering)

On the other hand, do we not sort tape wires by default because we can have custom wire labels like strings?

Additional information

This behaviour was highlighted in this PR: https://github.com/unitaryfund/mitiq/pull/836

Source code

import pennylane as qml

with qml.tape.QuantumTape() as tape:
    qml.PauliX(wires=1)
    qml.PauliY(wires=0)

print("Default wire ordering:\n")
print(tape.to_openqasm())

print("\nCorrected wire ordering:\n")
print(tape.to_openqasm(wires=sorted(tape.wires)))

Tracebacks

No response

System information

Dev PL

[X] I have searched exisisting GitHub issues to make sure the issue does not already exist.

josh146 commented 3 years ago

@trbromley this is expected behaviour, unfortunately! As you say, the tapes wires, since the wire refactor, have no inherent order; if you want a specific order, you need to pass wire_order.

If not provided, the default wire order is simply assumed to be the wire order of operations on the tape; hence why in this case you have 1, 0. We considered sorting in some cases, but I think we ran into a lot of edge cases, @mariaschuld?

Note that this is mitigated somewhat at the QNode level. Since QNodes are always composed of a tape and a device, the device provides the wire order information. E.g., consider the qml.draw(qnode) function:

def draw(qnode, charset="unicode", wire_order=None, show_all_wires=False):

    @wraps(qnode)
    def wrapper(*args, **kwargs):
        qnode.construct(args, kwargs)
        _wire_order = wire_order or qnode.device.wires
        _wire_order = qml.wires.Wires(_wire_order)

        if show_all_wires and len(_wire_order) < qnode.device.num_wires:
            raise ValueError(
                "When show_all_wires is enabled, the provided wire order must contain all wires on the device."
            )

        if not qnode.device.wires.contains_wires(_wire_order):
            raise ValueError(
                f"Provided wire order {_wire_order.labels} contains wires not contained on the device: {qnode.device.wires}."
            )

        return qnode.qtape.draw(
            charset=charset, wire_order=_wire_order, show_all_wires=show_all_wires
        )

    return wrapper

rmlarose commented 3 years ago

Thanks all! For me the other unexpected behavior is that tape.to_openqasm() measures all qubits even though no measurements may be present. E.g., the first example of https://github.com/PennyLaneAI/pennylane/issues/1500#issue-961796761:

import pennylane as qml

with qml.tape.QuantumTape() as tape:
    qml.PauliX(wires=1)
    qml.PauliY(wires=0)

>>> print(tape.to_openqasm())
OPENQASM 2.0;
include "qelib1.inc";
qreg q[2];
creg c[2];
x q[0];
y q[1];
measure q[0] -> c[0];
measure q[1] -> c[1];

Thoughts on only measuring the qubits in QASM that are measured in the tape?

josh146 commented 3 years ago

For me the other unexpected behavior is that tape.to_openqasm() measures all qubits even though no measurements may be present

@rmlarose that is a good point! We can definitely alter this behaviour. I'm not 100% sure why all qubits are currently measured; it could be one of two things:

An oversight when we first implemented this
Or, one of the hardware backends required that all qubits be measured(?), which perhaps biased our implementation.

albi3ro commented 3 years ago

Since even though this is non-intuitive behaviour, it is expected, should we go ahead and close the issue without a fix? @josh146 @trbromley

trbromley commented 3 years ago

For me this links to a more general question of wires being maybe too flexible (e.g., functions, floats) such that we can't sort them. If that were solved, it'd be easy to fix this issue.

In terms of closing this, I think it's still a valid issue in the sense that it's doing something weird even if its what we expect at this point. Maybe we need a "known issues" category of stuff we're not going to worry about, but still keep track of. Otherwise, if you find this is unnecessary noise in the list of issues, then it's also ok to close.

PennyLaneAI / pennylane