Raise an error when we return `sample()` or `counts()` with observables that are not diagonal!

[Jaybsoni]

Context: sample and counts are measurement processes which admit a special syntax: sample(), counts(). We interpret this syntax to mean "return computational basis samples or counts". However, this measurement is not commutative with other measurements which are not diagonal in the computational basis. Thus they are not allowed to be performed simultaneously.

We now raise an error when the user tries to measure these simultaneously with other non-commuting measurements.

Description of the Change: Add check in execute() to determine if there are any diagonalizing gates and if we are also asking for computational basis samples.

Related GitHub Issues: https://github.com/PennyLaneAI/pennylane/issues/2863

[Jaybsoni]

[sc-23709]

[github-actions[bot]]

Hello. You may have forgotten to update the changelog! Please edit doc/releases/changelog-dev.md with:

• A one-to-two sentence description of the change. You may include a small working example for new features.
• A link back to this PR.
• Your name (or GitHub username) in the contributors section.

[Jaybsoni]

@antalszava and @rmoyard,

It seems some of the tests for the new return types are failing due to this bug fix. It seems that in these tests you are actually measuring things that "can't" be measured together. Let me know what you think I should do to account for this? I can also just remove this update from the new execute method and you can add it in the respective PR specifically.

[Jaybsoni]

I don't think that's the right way to go here. We have a system to support non commutative observables by batching the tapes. Normally it should split the tapes between the non-commuting part and creates multiple tapes. For sample it is not supported at the moment. We have the following check in split_non_commuting.py

    if qml.measurements.Sample in return_types or qml.measurements.Probability in return_types:
        raise NotImplementedError(
            "When non-commuting observables are used, only `qml.expval` and `qml.var` are supported."
        )

It is called in batch_transform in _device.py. Imo it would make more sense to update split_commuting and batch_transform in order to support Sample and Counts. I think it implies changing the tapes._obs_sharing_wires as well.

@rmoyard I agree! This is what I mentioned in the meeting that the better / long term solution is to support Sample and Counts in split_commuting itself. But this requires more work and @Qottmann mentioned there are some non-trivial aspects which prevented him from including it initially.

We discussed in the short term to raise an error as without the error users would be able to execute this and the wrong samples / counts with no indication that it's wrong. I initially thought about raising the error at the tape level (in expand_tape, but this would raise an error for any tape, independent of the device. This isn't a good solution because there may be other devices for which it is perfectly fine to do such a measurement. In my opinion it makes most sense that this error be raised on the device level, as that's where the non-commuting measurement will take place.

[AlbertMitjans]

I don't think that's the right way to go here. We have a system to support non commutative observables by batching the tapes. Normally it should split the tapes between the non-commuting part and creates multiple tapes. For sample it is not supported at the moment. We have the following check in split_non_commuting.py

    if qml.measurements.Sample in return_types or qml.measurements.Probability in return_types:
        raise NotImplementedError(
            "When non-commuting observables are used, only `qml.expval` and `qml.var` are supported."
        )

It is called in batch_transform in _device.py. Imo it would make more sense to update split_commuting and batch_transform in order to support Sample and Counts. I think it implies changing the tapes._obs_sharing_wires as well.

@rmoyard these checks are already done before calling this function: https://github.com/PennyLaneAI/pennylane/blob/2582fcfd8ce633fb53fc5868b4e8950c7dbc810f/pennylane/_device.py#L736

Do you think we could remove them?

[Jaybsoni]

As you mentioned right now we are allowing non-commuting measurements when using qml.sample() and qml.counts() without any observable. However when using an observable this is not allowed:

import pennylane as qml
dev = qml.device("default.qubit", wires=4, shots=1000)
@qml.qnode(dev)
def circuit():
    return qml.expval(qml.PauliX(0)), qml.sample(qml.PauliZ(0))
print(circuit())

Raises:

QuantumFunctionError: Only observables that are qubit-wise commuting Pauli words can be returned on the same wire.

My question is, could we re-use this Error for the non-observable case?

Using qml.counts() and qml.sample() with wires=None and obs=None should be equivalent to sampling all wires on the basis state, right? But this is not represented in the MeasurementProcess class, given that wires and obs are still None:

>>> meas = qml.counts() 
>>> meas.obs 
>>> meas.wires 
<Wires = []>

Consequently, these measurements are omitted in the _update_observables method of the QuantumTape class, and tape._obs_sharing_wires is kept empty

https://github.com/PennyLaneAI/pennylane/blob/2582fcfd8ce633fb53fc5868b4e8950c7dbc810f/pennylane/tape/tape.py#L517

and python skips the check for commuting observables: https://github.com/PennyLaneAI/pennylane/blob/1f149fcf316b6e62dd9be17aceb9619e42f8ed73/pennylane/_device.py#L654

https://github.com/PennyLaneAI/pennylane/blob/436d85c96f37c6162d94371c30659101cbd9426e/pennylane/tape/tape.py#L159

All in all, wouldn't it be easier to (somehow) set wires=all_wires and obs=qml.PauliZ when using qml.counts() and qml.sample? Then we would obtain the commutativity check for free.

Maybe this is not in scope, but I realised that running the code above with qml.counts doesn't raise an error. I will try to figure out why.

Edit: Found the source of the bug: when using qml.counts, split_non_commuting was called and changed qml.counts to qml.expval. To solve this an if qml.measurements.Counts not in return_types check must be added here:

https://github.com/PennyLaneAI/pennylane/blob/2582fcfd8ce633fb53fc5868b4e8950c7dbc810f/pennylane/_device.py#L736

I added this in PR #2928.

This is also a good idea @AlbertMitjans. One issue would be that when we provide an observable (say PauliZ), the raw samples (which will be 0s, or 1s) get transformed to be samples of the eigenvalues of the observable (-1s and 1s in this case). So doing something like what you suggested on the level of measurement.py where we can replace in the measurement process obs=None, wires=None --> obs=qml.PauliZ(wires=0)@qml.PauliZ(wires=1) @ ... wouldn't replicate the behaviour because then we would have no way of checking when to return raw samples vs. when to return eigenvalue samples.

[AlbertMitjans]

This is also a good idea @AlbertMitjans. One issue would be that when we provide an observable (say PauliZ), the raw samples (which will be 0s, or 1s) get transformed to be samples of the eigenvalues of the observable (-1s and 1s in this case). So doing something like what you suggested on the level of measurement.py where we can replace in the measurement process obs=None, wires=None --> obs=qml.PauliZ(wires=0)@qml.PauliZ(wires=1) @ ... wouldn't replicate the behaviour because then we would have no way of checking when to return raw samples vs. when to return eigenvalue samples.

What about just setting the m.wires to "all_wires" and then removing the if m.obs is None in the following line? Or is this check used for something else? https://github.com/PennyLaneAI/pennylane/blob/2582fcfd8ce633fb53fc5868b4e8950c7dbc810f/pennylane/tape/tape.py#L517

I am insisting on this because I find it inconsistent that when using qml.sample(), which corresponds to a measurement on all wires, the attribute tape._obs_sharing_wires does not take this into account. And it might also introduce more inconsistencies in the future.