qiskit-community / qiskit-nature

Qiskit Nature is an open-source, quantum computing, framework for solving quantum mechanical natural science problems.
https://qiskit-community.github.io/qiskit-nature/
Apache License 2.0
301 stars 206 forks source link

VQEResult() is incompatible with ElectronicStructureProblem.interpret() #1024

Closed MarcoBarroca closed 1 year ago

MarcoBarroca commented 1 year ago

Environment

What is happening?

When doing VQE calculations with the Estimators you might get results of type OptimizerResult(), one might build a VQEResult() and MinimumEigensolverResult() object from it like so:

class CustomVQE(MinimumEigensolver):

    def __init__(self, estimator, circuit, optimizer, callback=None):
        self._estimator = estimator
        self._circuit = circuit
        self._optimizer = optimizer
        self._callback = callback
        self._eval_count=0

    def compute_minimum_eigenvalue(self, operators,initial_point, aux_operators=None):

        def objective(x):

            job = self._estimator.run([self._circuit], [operators], [x])
            est_result = job.result()
            value = est_result.values[0]
            std=est_result.metadata[0]
            self._eval_count+=1
            if self._callback is not None:
                self._callback(self._eval_count, value, std)
            return value    
        x0 =initial_point

        # Optimize
        res = self._optimizer.minimize(objective, x0=x0)

        # Populate VQE result
        vqe_result = VQEResult()
        vqe_result.cost_function_evals = res.nfev
        vqe_result.eigenvalue = res.fun
        vqe_result.optimal_parameters = res.x
        eigen_result=MinimumEigensolverResult()
        eigen_result.eigenvalue=res.fun
        return vqe_result, eigen_result, res

ElectronicStructureProblem.interpret() is only compatible with the MinimumEigensolverResult(). Isn't this counterintuitive as that object can't store information like the number of evaluations or optimal parameters? This forces the user to either create multiple result objects to keep everything or manually interpret results by adding values from the Active Space transformation for instance.

How can we reproduce the issue?

Including the code above should create the errors. We need need to create a problem.

#Callback Function

counts = []
values = []
deviation = []
def callback(eval_count, mean, std):  
    # Overwrites the same line when printing
    display("Evaluation: {}, Energy: {}, Std: {}".format(eval_count, mean, std))
    clear_output(wait=True)
    counts.append(eval_count)
    values.append(mean)
    deviation.append(std)

#VQE program

molecule = MoleculeInfo(
             # coordinates in Angstrom
                     symbols=['Li','F'],
                     coords=[
                            (0.0 ,0.0 ,0.0),
                            (0.0 ,0.0 ,1.672)
                            ],
                     multiplicity=1,  # = 2*spin + 1
                     charge=0,
                     units=DistanceUnit.ANGSTROM
                    )

#Set driver
driver = PySCFDriver.from_molecule(molecule, basis="sto3g", method=MethodType.RKS)

og_problem = driver.run()

transformer = ActiveSpaceTransformer(
    num_electrons=(6,6), #Electrons in active space
    num_spatial_orbitals=10 #Orbitals in active space
    #,active_orbitals=np.arange(1,11,1)
)

#Define the problem

problem=transformer.transform(og_problem)

num_spatial_orbitals = problem.num_spatial_orbitals
num_particles = problem.num_particles

hamiltonian=problem.hamiltonian
second_q_op = hamiltonian.second_q_op() 

mapper=ParityMapper()

converter = QubitConverter(mapper)

#Final OP
qubit_op = converter.convert(second_q_op,num_particles=num_particles,sector_locator=problem.symmetry_sector_locator)

optimizer = SLSQP(maxiter=3)
init_state = HartreeFock(num_spatial_orbitals, num_particles, converter)
ansatz = EfficientSU2(num_qubits=qubit_op.num_qubits,su2_gates='ry', entanglement='linear', reps=3, initial_state=init_state)
initial_point = np.pi/4 * np.random.rand(ansatz.num_parameters)

estimator = Estimator([ansatz], [qubit_op])
custom_vqe=CustomVQE(estimator, ansatz, optimizer, callback=callback)

vqe_result,eigen_result,res = custom_vqe.compute_minimum_eigenvalue(qubit_op,initial_point=initial_point)

problem.interpret(res) #This returns the error: "TypeError: Cannot construct an EigenstateResult from a result of type, <class 'qiskit.algorithms.optimizers.optimizer.OptimizerResult'>."

problem.interpret(vqe_result) #This returns the error: "Cannot construct an EigenstateResult from a result of type, <class 'qiskit.algorithms.minimum_eigen_solvers.vqe.VQEResult'>."

problem.interpret(res) #This gives no error

What should happen?

Ideally, no errors would happen and problem.interpret() would handle other types of results as all of those objects carry the eigenvalue information.

Any suggestions?

No response

woodsp-ibm commented 1 year ago

ElectronicStructureProblem.interpret() is only compatible with the MinimumEigensolverResult(). Isn't this counterintuitive as that object can't store information like the number of evaluations or optimal parameters?

The algorithms used are (Minimimum)Eigensolvers giving (Minimum)EigensolverResults - this is the level things are common. Classes implementing this can extend the result (and behavior), like VQE does, and add additional information etc. E.g. I can add a new solver and if it say does stretches add the number of stretches in the result. Interpreting any algorithm additional information would be algorithm specific if indeed it makes any sense to interpret - interpret here really more means if we made transformations to the problem interpret the result back to the user level. So for ground state it can add back in frozen core energy, nuclear repulsion etc for total ground state energy as VQE computes the residual electron part. How many iterations it took may be of interest to a user but its not a solution to the problem for interpretation. Now the underlying result that includes that information is provide in raw_result fields. The common parts of that - the (Minimum)EigensolverResult defined fields, are understood and used in the interpretation - but you are free to access any of the other fields.

mrossinek commented 1 year ago

There is indeed an implicit hierarchy of the result objects spanning across the Qiskit stack. This is not ideal and I agree that we might be able to improve that by supporting more result types.

However, before doing anything in this direction I think it would be important that we finally refactor the result classes to actually be useful. As they are now, they are storage containers which exist at runtime and can be formatted as a string. However, they have no means of being stored in a file or converted to a dictionary or whatever. This is quite a severe limitations in their usability in my opinion.

Ideally, we would refactor these classes as dataclasses but so far we have stayed away from these because of some limitations in the Qiskit documentation theme. This is being worked on (afaik) and we should strive towards a general refactoring and improvement of the result objects in the Qiskit stack. That said we might need to start with the lower level result objects from Terra before refactoring the ones in Nature.

woodsp-ibm commented 1 year ago

In looking at the code, why its failing for VQEResult is that I think you imported the old one, and not the one from the new Estimator based VQE it says

problem.interpret(vqe_result) #This returns the error: "Cannot construct an EigenstateResult from a result of type, <class 'qiskit.algorithms.minimum_eigen_solvers.vqe.VQEResult'>."

class 'qiskit.algorithms.minimum_eigen_solvers.vqe.VQEResult'

But it should be of type qiskit.algorithms.minimum_eigensolvers.VQEResult which extends qiskit.algorithms.minimum_eigensolvers.MinimumEigensolverResult (there is no underscore in eigen_solvers - a new folder was created with new implementations supporting the new primitives and yes it did double up on the names which is why for now they have to imported direct from these folders for the new algos etc rather than qiskit.algorithms which is still the original algorithms using quantum instance)

MarcoBarroca commented 1 year ago

I was importing as from qiskit.algorithms import VQEResult , which as you said is pulling from the old minimum_eigen_solvers one. Forcing the import to the new one does indeed solve the issue.

Sorry if I mislabeled this as a bug, I just wasn't sure how to point it out.

woodsp-ibm commented 1 year ago

Sorry if I mislabeled this as a bug, I just wasn't sure how to point it out.

No worries at all - not a problem. Its not uncommon for us to re-label an issue.