BugFix - Returning sums of observables on the same wires provides incorrect results

[PietropaoloFrisoni]

Context:

These are the codes that detected this bug in the first place. I report the current (correct) result and the previous (incorrect) result:

import numpy as np
import pennylane as qml

dev = qml.device("default.qubit", wires=2, shots=1000)

@qml.qnode(dev)
def circuit():
    qml.RX(np.pi / 2, 0)
    qml.RX(-np.pi / 2, 1)
    return qml.expval(qml.Y(0) + qml.Y(0)), qml.expval(qml.Y(1))

>>> circuit()
(2.0, 1.0)  # previous result: (2.0,)

@qml.qnode(dev)
def circuit():
    qml.RX(np.pi / 2, 0)
    qml.RX(-np.pi / 2, 1)
    return qml.expval(qml.Y(1)), qml.expval(qml.Y(0) + qml.Y(0))

>>> circuit()
(1.0, -2.0)  # previous result: (1.0, 2.0)

The bug is caused by the current logic in the qml.devices.qubit.measure_with_samples function.
This function internally calls the _group_measurements function and selects a different measure function for each group returned by the former.

From now on, we assume that qml.devices.qubit is the current directory.

The grouping strategy in the _group_measurements function is such that _measure_sum_with_samples is the selected measurement function when a sum is involved in the group. Unfortunately, the logic in the grouping process and _measure_sum_with_samples cannot correctly handle the case in which there are multiple measurement processes and one involves a sum of operators acting on the same wire.

I tried several different solutions, noticing that:

1. Many functions in the source code depend on the measure_with_samples function. Any 'major change' to this function results in test failures across PL. Re-designing the entire logic of this function is beyond the scope of this PR.

2. Changing just the grouping logic in _group_measurements or the workflow in _measure_sum_with_samples cannot really solve this issue, since the logic of the entire function is 'entangled' among all its components.

There is a way to save goats and cabbages.

Description of the Change:

In the _group_measurements function, we simplify the measurement processes involving a sum. This leaves untouched the cases for which the function was originally designed, but converts the sum of operators acting on the same wires into a SProd. Therefore, the logic in _group_measurements becomes compatible with the workflow of the measure_with_sample function.

That is, _measure_with_samples_diagonalizing_gates is the chosen measurement function in place of _measure_sum_with_samples. The latter is still the selected measurement function for sums of different operators and/or sums of the same operator acting on the different wires (I suspect these were the cases taken into account by the original author in the first place).

Benefits: The function now returns the correct result.

Possible Drawbacks: We are simply re-routing an internal possible workflow of the measure_with_samples function to make it compatible with the logic intended by the author. Although everything is possible, it is unlikely that any major problem is caused directly by this PR.

Related GitHub Issues: #5976

Related ShortCut Stories [sc-68218]

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.67%. Comparing base (7174beb) to head (c9d6ceb).
Report is 283 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5978      +/-   ##
==========================================
- Coverage   99.67%   99.67%   -0.01%     
==========================================
  Files         425      425              
  Lines       40924    40630     -294     
==========================================
- Hits        40791    40496     -295     
- Misses        133      134       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[lillian542]

Looks good to me, thanks for tracking this down @PietropaoloFrisoni 🎉

[astralcai]

Potentially we can re-use split_non_commuting to replace this completely at some point.

[PietropaoloFrisoni]

Potentially we can re-use split_non_commuting to replace this completely at some point.

At some point, I agree. It seems to me that this would require changing the grouping strategy and the current logic of measure_with_sampling. This is a simple bugfix to return the correct result, so it's is beyond the purpose of the PR 👍

[astralcai]

LGTM