Deprecation of BaseSamplerV1 has a conflict with HamiltonianPhaseEstimation class

emamars95 commented 1 month ago

Environment

Qiskit version: 1.2.1
Python version: 3.12.0
Operating system: Ubuntu 22.04.4

What is happening?

Since the release of the version Qiskit 1.2 the class Sampler is deprecated. With this choice we require that all implementations of the BaseSamplerV1 interface have been deprecated in favour of their V2 counterparts. The V2 alternative for the Sampler class is StatevectorSampler.

The class HamiltonianPhaseEstimation has not be modified to accommodate this modification and still uses the BaseSamplerV1 Sampler class. At the moment, the code is still working with the exception of printing a DeprecationWarning. However, once the Sampler class will be removed it will generate an error.

At the moment StatevectorSampler CANNOT be used as sampler for the HamiltonianPhaseEstimation. This is due to the implementation of the function estimate_from_pe_circuit in the module qiskit_algorithms/phase_estimators/phase_estimation.py.

How can we reproduce the issue?

Import required modules

import qiskit 
import qiskit_algorithms
from qiskit.primitives import Sampler, StatevectorSample
from qiskit.quantum_info import Statevector, SparsePauli
from qiskit_algorithms import HamiltonianPhaseEstimation

Use the Sampler class to run the HamiltonianPhaseEstimation

sampler = Sampler(options={"shots" : 10, "seed" : 0})
qpe = HamiltonianPhaseEstimation(num_evaluation_qubits=2, 
                                 sampler=sampler)

qpe_result = qpe.estimate(hamiltonian=SparsePauliOp('XZ'),
                          state_preparation=Statevector.from_label("01"))

/tmp/ipykernel_1033530/360718787.py:1: DeprecationWarning: The class qiskit.primitives.sampler.Sampler is deprecated ...

Use the StatevectorSampler leads to an error:

bound = 0.8
sampler = StatevectorSampler(default_shots=10, seed=0)
qpe = HamiltonianPhaseEstimation(num_evaluation_qubits=2, 
                                 sampler=sampler)

qpe_result = qpe.estimate(hamiltonian=SparsePauliOp('XZ'),
                          state_preparation=Statevector.from_label("01"))

--> 6 qpe_result = qpe.estimate(hamiltonian=SparsePauliOp('XZ'),

File ~/qiskit_algorithms/phase_estimators/hamiltonian_phase_estimation.py:173, in HamiltonianPhaseEstimation.estimate(self, hamiltonian, state_preparation, evolution, bound)

--> 173 phase_estimation_result = self._phase_estimation.estimate( 174 unitary=unitary, state_preparation=state_preparation)

File ~/qiskit_algorithms/phase_estimators/phase_estimation.py:230, in PhaseEstimation.estimate(self, unitary, state_preparation)

--> 230 return self.estimate_from_pe_circuit(pe_circuit)

File ~/qiskit_algorithms/phase_estimators/phase_estimation.py:199, in PhaseEstimation.estimate_from_pe_circuit(self, pe_circuit)

--> 199 phases = circuit_result.quasi_dists[0]

AttributeError: 'PrimitiveResult' object has no attribute 'quasi_dists'

The reason of the error arise because the BaseSamplerV1 provides a result that is a quasi distribution, while BaseSamplerV2 return a primitive unified blocs (PUB) results which is inconsistent with the actual implementation.

What should happen?

The estimate_from_pe_circuit function in phase_estimation.py should check which Sampler is passed on and allow back-compatibility, in case it is passed the BaseSamplerV1, and also allow the usage of the suggested BaseSamplerV2

Any suggestions?

I am happy to open a pull request to modify the code from the current implementation

phases = circuit_result.quasi_dists[0]
phases_bitstrings = {}
for key, phase in phases.items():
    bitstring_key = self._get_reversed_bitstring(self._num_evaluation_qubits, key)
    phases_bitstrings[bitstring_key] = phase
phases = phases_bitstrings
return PhaseEstimationResult(
    self._num_evaluation_qubits, circuit_result=circuit_result, phases=phases
)

To my suggested implementation:

phases_bitstrings = {}
if isinstance(self._sampler, BaseSampler): 
    print("sampler V1")
    phases = circuit_result.quasi_dists[0]
    for key, phase in phases.items():
        bitstring_key = self._get_reversed_bitstring(self._num_evaluation_qubits, key)
        phases_bitstrings[bitstring_key] = phase
elif isinstance(self._sampler, BaseSamplerV2): 
    phases = circuit_result[0].data.meas.get_counts()
    for key, phase in phases.items():
        phases_bitstrings[key[::-1]] = phase
else: 
    raise ValueError("Sampler does not belong to any known Sampler class")
phases = phases_bitstrings
return PhaseEstimationResult(
    self._num_evaluation_qubits, circuit_result=circuit_result, phases=phases
)

Further modification is the annotation of the classes as sampler: BaseSampler | BaseSamplerV2 | None = None. I am open to discuss wheatear the check of the Sampler type is more appropriate in the initialization of the PhaseEstimation object.

jakelishman commented 1 month ago

qiskit_algorithms is a separate package (and sadly doesn't have much maintainer activity anymore) - there's nothing we can do about it from this repository. The right repository is https://github.com/qiskit-community/qiskit-algorithms, but I should warn you that there's not much activity, since it's no longer supported by the core Qiskit team.

emamars95 commented 1 month ago

Yes, I actually wanted to post the issue in qiskit-algorithms. I noticed I chose in the wrong repo only after posting the issues (never do this kind of things at the end day 😂).

I see. I will open a pull request there and see if someone can moderate the request with me. I will ask the community there if they need another pair of eyes to help them out. Thanks for the heads up anyway!

Qiskit / qiskit