unitaryfund / mitiq

Mitiq is an open source toolkit for implementing error mitigation techniques on most current intermediate-scale quantum computers.
https://mitiq.readthedocs.io
GNU General Public License v3.0
363 stars 160 forks source link

Expectation values of Mitiq observables are buggy if circuits have measurements #1420

Closed andreamari closed 2 years ago

andreamari commented 2 years ago

Issue Description

Expectation values of Mitiq observables are buggy if circuits have measurements It seems that change of basis rotations are applied after measurement gates instead of before.

How to Reproduce

Code Snippet

from mitiq import Observable, PauliString
from qiskit import QuantumCircuit

obs = Observable(PauliString("X"))
qiskit_circuit = QuantumCircuit(1)
qiskit_circuit.h(0)
qiskit_circuit.measure_all()

print("Input circuit")
print(qiskit_circuit)
print(f"Circuit to measure {obs}")
print(obs.measure_in(qiskit_circuit)[0])

Output

Input circuit
        ┌───┐ ░ ┌─┐
     q: ┤ H ├─░─┤M├
        └───┘ ░ └╥┘
meas: 1/═════════╩═
                 0 
Circuit to measure X(q(0))
            ┌───┐┌─┐┌──────────┐┌─┐
         q: ┤ H ├┤M├┤ Ry(-π/2) ├┤M├
            └───┘└╥┘└──────────┘└╥┘
m_meas_0: 1/══════╩══════════════╬═
                  0              ║ 
                                 ║ 
      m0: 1/═════════════════════╩═```

Expected Output

The Circuit to measure X(q(0)) should have a single measurement gate applied after the Ry rotation (instead of two measurement gates).

Environment Context

Use the about() function to summarize information on operating system, python version and dependencies.

Mitiq: A Python toolkit for implementing error mitigation on quantum computers
==============================================================================
Authored by: Mitiq team, 2020 & later (https://github.com/unitaryfund/mitiq)

Mitiq Version:  0.18.0dev

Core Dependencies
-----------------
Cirq Version:   0.15.0
NumPy Version:  1.20.3
SciPy Version:  1.7.3

Optional Dependencies
---------------------
PyQuil Version: 3.0.1
Qiskit Version: 0.36.2
Braket Version: 1.26.2

Python Version: 3.7.7
Platform Info:  Linux (x86_64)
andreamari commented 2 years ago

Assigned to @nickdgardner , thanks!

rmlarose commented 2 years ago

This is the intended behavior - measure_in adds single-qubit rotations and measurements to the end of a circuit, regardless of what the circuit is. There could be use cases where you want intermediate measurements in the circuit. Where did this come up? and what is the proposed change?

nickdgardner commented 2 years ago

@andreamari @rmlarose I was looking into the issue but I'll hold off until this question is resolved.

andreamari commented 2 years ago

This is the intended behavior - measure_in adds single-qubit rotations and measurements to the end of a circuit, regardless of what the circuit is. There could be use cases where you want intermediate measurements in the circuit. Where did this come up? and what is the proposed change?

The situation that I had in mind is the the quite typical setting in which there are just terminal measurements in the input circuit and I expected that one could evaluate expectation values of arbitrary observables (by changing the basis before the measurements).

But I also understand your design view and it makes sense to me. In practice this means that, when using a Mitiq obervable, in most cases the user should just define circuits without measurements since Mitiq will take care of appending them.

@nickdgardner, sorry if you spent time on this. I am closing this issue since it refers to a bug that is not a bug.

It may still be worth adding a warning in the docs about terminal measurements + observable or maybe an actual Python warning in measure_in when the circuit has pre-existing terminal measurements. If you are interested, feel free to make a new issue and/or a new PR about ways of warning the user.

rmlarose commented 2 years ago

Thanks Andrea, that all SGTM. +1 to a note in documentation, probably the observables doc. +0.5 to a warning - I'd like to see a larger user base experiencing this issue before it's actually implemented.

The situation that I had in mind is the the quite typical setting in which there are just terminal measurements in the input circuit and I expected that one could evaluate expectation values of arbitrary observables (by changing the basis before the measurements).

Yes, the problem here is the observables wouldn't be arbitrary then - they would have to have support on the measured qubits. Or you would have to check if there's already a measurement on some qubits and not on others. Both of these are messes.

nickdgardner commented 2 years ago

@andreamari No worries it was still useful for learning the code base--I'll do an issue + PR to add a warning in the docs about this.

andreamari commented 2 years ago

Great, thanks @nickdgardner!

natestemen commented 2 years ago

Just came across the test which references this issue: https://github.com/unitaryfund/mitiq/blob/6af27485859e1b284d2607d5ef901078a303931b/mitiq/interface/mitiq_qiskit/tests/test_qiskit_utils.py#L298-L320

The test fails locally (which makes sense this no action was taken here)

$ pytest pytest mitiq/interface/mitiq_qiskit/tests/test_qiskit_utils.py::test_compute_expectation_value_on_noisy_backend_with_measurements

mitiq/interface/mitiq_qiskit/tests/test_qiskit_utils.py F                                                                                                  [100%]

============================================================================ FAILURES ============================================================================
_______________________________________________ test_compute_expectation_value_on_noisy_backend_with_measurements ________________________________________________

    def test_compute_expectation_value_on_noisy_backend_with_measurements():
        """Tests the evaluation of an expectation value when the input
        circuit has measurements.
        """
        obs = Observable(PauliString("X"))
        qiskit_circuit = QuantumCircuit(1)
        qiskit_circuit.h(0)
        qiskit_circuit.measure_all()

>       expval = compute_expectation_value_on_noisy_backend(
            qiskit_circuit,
            obs,
            noise_model=initialized_depolarizing_noise(0.01),
        )

mitiq/interface/mitiq_qiskit/tests/test_qiskit_utils.py:307:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
mitiq/interface/mitiq_qiskit/qiskit_utils.py:298: in compute_expectation_value_on_noisy_backend
    return executor.evaluate(circuit, obs)[0]
mitiq/executor/executor.py:190: in evaluate
    all_results = self._run(all_circuits, force_run_all, **kwargs)
mitiq/executor/executor.py:265: in _run
    self._call_executor(circuit, **kwargs)
mitiq/executor/executor.py:295: in _call_executor
    result = self._executor(to_run, **kwargs)  # type: ignore
mitiq/interface/mitiq_qiskit/qiskit_utils.py:252: in sample_bitstrings
    bitstring = [int(c) for c in key]
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

.0 = <str_iterator object at 0x1773cfcd0>

>   bitstring = [int(c) for c in key]
E   ValueError: invalid literal for int() with base 10: ' '

mitiq/interface/mitiq_qiskit/qiskit_utils.py:252: ValueError

Do we want to remove the test now?

andreamari commented 1 year ago

Ok for removing the test! Good catch