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
291 stars 197 forks source link

Inconsistent behavior of `HartreeFock` and `UCC` for fully occupied orbitals #1277

Closed julenl closed 7 months ago

julenl commented 8 months ago

Environment

What is happening?

Trying to run a UCC calculation on noble gas dimmers does not seem to work.

How can we reproduce the issue?

from qiskit_nature.second_q.drivers import PySCFDriver

driver = PySCFDriver( atom='He 0 0 0; He 0 0 5.2' )
problem = driver.run()

from qiskit_nature.second_q.mappers import JordanWignerMapper
mapper = JordanWignerMapper()

from qiskit_algorithms.optimizers import SLSQP
from qiskit_algorithms import VQE
from qiskit.primitives import Estimator
from qiskit_nature.second_q.circuit.library import HartreeFock, UCCSD

ansatz = UCCSD(
    problem.num_spatial_orbitals,
    problem.num_particles,
    mapper,
    initial_state=HartreeFock(
        problem.num_spatial_orbitals,
        problem.num_particles,
        mapper,
    ),
)

print(ansatz.num_parameters)

Returns:

ValueError: The number of spatial orbitals 2must be greater than number of particles of any spin kind (2, 2).

What should happen?

The ansatz should work also for fully occupied orbitals.

Any suggestions?

I have noticed that there is some incoherence between two similar validations:

The text on the second one says "greater than or equal" but the condition only has a ">", and it works. But the ucc one says "greater than" and the condition is ">=", so it does not work for any noble gas.

I tried removing the "=" from the ">=" for ucc, but then the operator turns for some reason into a list at some point and crashes. 

mrossinek commented 8 months ago

Besides the fact that, when both spin registers (alpha and beta) are fully occupied, UCCSD is not really going to give you an improved result, I agree that the exception logic around this could be improved a bit. Especially, since this failure also occurs when e.g. only the alpha spin register is completely filled while the beta-spin is not. I ran into the past and in such cases simply disabled the check because the code works fine (for when at least the beta-spin is not fully occupied).

Sidenote: I disabled the check by removing the private method which runs the check from the class instance (i.e. by overwriting it with a lambda function which always returns True)

I tried removing the "=" from the ">=" for ucc, but then the operator turns for some reason into a list at some point and crashes.

The "problem" is, that a filled register leads to empty lists of excitations which might need to be handled differently in pieces of the code. But that should not be difficult to fix.

Would you be interested in opening a PR which includes the following?

julenl commented 8 months ago

I'll be more than happy to try, but I will need some assistance.