Executors fail to recognize observable when function is not typehint

[natestemen]

The following snippet only works when the execute function has a typehint for the functions return type.

import networkx as nx

from mitiq import MeasurementResult, benchmarks, rem
from mitiq.observable.observable import Observable
from mitiq.observable.pauli import PauliString

depth, num_qubits = 10, 4
circuit, bs = benchmarks.generate_mirror_circuit(
    nlayers=depth,
    two_qubit_gate_prob=1.0,
    connectivity_graph=nx.complete_graph(num_qubits),
    return_type="qiskit",
)

def execute(circuit, shots=100) -> MeasurementResult:
    fake_bitstrings = [[0, 0, 1, 1] for _ in range(shots)]

    return MeasurementResult(fake_bitstrings)

p_flip = 0.05
icm = rem.generate_inverse_confusion_matrix(len(bs), p_flip, p_flip)
obs = Observable(PauliString("ZZZZ"))

rem_value = rem.execute_with_rem(
    circuit,
    execute,
    obs,
    inverse_confusion_matrix=icm,
)

print(f"Expectation Value After REM: {rem_value}")

The error that is raised when removing the typehint is

ValueError: Executor and observable are not compatible. Executors
                returning expectation values as float must be used with
                observable=None

which comes from the Executor class. In particular a check to ensure the observable and return type of the executor are compatible. https://github.com/unitaryfund/mitiq/blob/231319df9051dfbc61db79d02bee201979528a36/mitiq/executor/executor.py#L163-L172

---

Needless to say, this code should work regardless of the typehint. If investigation shows that the typehint truly is needed, then this requirement needs to be documented.

[bdg221]

@natestemen, I looked into this issue and it appears to be an issue when the Executor.evaluate function is called with an observable. This means that it applies to ZNE, REM, PEC, CDR, DDD, and QSE.

When a function is passed into any of the execute_with_XXX, eventually the function is used to create an Executor object. During that instantiation, the _executor_return_type is set.

self._executor_return_type = executor_annotation.get("return")

The executor_annotation.get call pulls from the typehint. If that is not set, then the _executor_return_type is None.

This comes back to bite us in Executor.evaluate when determining how to set all_circuits as this does the following check in the code:

if (observable is not None   and   self._executor_return_type in MeasurementResultLike)
...
elif(observable is not None   and    self._executor_return_type not in MeasurementResultLike
            and    self._executor_return_type not in DensityMatrixLike)
...
else

During this part of the code, it is assumed that if _executor_return_type is not in MeasurementResultLike and not in DensityMatrixLike it MUST be in FloatLike (as opposed to None).

If I correct the else if to be the following:

        elif (
            observable is not None
            and self._executor_return_type in FloatLike
        ):

Then not using the typehint would result in the else branch being taken and the following code being run:

            all_circuits = circuits
            result_step = 1

Let me know your thoughts, but I see the solution to be:

• [ ] Update elif in Executor.evaluate to better handle not in MeasurementResultLike and DensityMatrixLike
• [ ] Determine if the code in else is sufficient (looks like a MeasurementResultLike would be more optimized, but without the typehint it will still run)
• [ ] This should be documented as either strongly recommended or required

[natestemen]

This means that it applies to ZNE, REM, PEC, CDR, DDD, and QSE.

Indeed, good catch. I've updated the issue title to reflect that fact.

---

After trying to write this comment for a full 2 days, I'm convinced their isn't a solution that satisfies everyone. There are two issues at play;

1. This issue was raised after a group was surprised by the need for type hinting the executor.
2. https://github.com/unitaryfund/mitiq/issues/1140 arose when Mitiq was silently providing strange results when an observable was passed with an incompatible executor.

I'm convinced resolving both of these issues without making a 'test call' to the users Executor, is impossible. I do think there are some improvements we can make here both in documentation and code.

Documentation

These suggestions are independent of code changes. The warnings should highlight what the needs are for our API.

1. We do mention in the The input function section of the Executors core concept page that type hinting or annotation is required. This should be upgraded to a warning[^directives] and flushed out.
2. The Using observables in error mitigation techniques section of the Observables core concept page should also have a warning and be flushed out.

Code

1. The measure_in method of the Observable class should raise an error when the function results in more than one measurement on a single qubit. I would almost always assume this is unintended behavior. This will not completely fix the problem, however, as someone may add measurements inside their executor function.
2. Modify the evaluate function of the Executor class to assume that if a user has passed an observable, then they are using a compatible Executor. I.e. remove this line, and the following elif block.

3. We should ensure our tests have cases that cover the following cases, adding them where needed.

   	mitigation method	executor type	type hinted	observable
   1.	ZNE	MeasurementResult	❌	✅
   2.	ZNE	MeasurementResult	✅	✅
   3.	ZNE	float	❌	❌[^test]
   4.	REM	MeasurementResult	❌	✅
   5.	REM	MeasurementResult	✅	✅

[^directives]: Details about admonitions can be found here: https://mystmd.org/guide/admonitions. [^test]: This is likely already tested somewhere, just a matter of finding it.

[FarLab]

I like this idea @natestemen, to assume the user has defined a compatible executor when passing an observable. If the executor is not compatible, the user gets an error down the road that should make it clear the executor is not compatible.

[bdg221]

2. Modify the evaluate function of the Executor class to assume that if a user has passed an observable, then they are using a compatible Executor. I.e. remove this line, and the following elif block.

Turns out the elif block can be removed, but the this line needs to stay because it adjusts the circuit for ONLY measurement like results. If the line is removed, then density matrix like results go into the if block and the circuits are modified. This broke all uses of density matrix simulators. 🤪