quantumlib / OpenFermion-PySCF

OpenFermion plugin to interface with the electronic structure package PySCF.
Apache License 2.0
104 stars 44 forks source link

run_pyscf does not work on PyscfMolecularData #38

Open kevinsung opened 6 years ago

kevinsung commented 6 years ago

Input:

from openfermionpyscf import PyscfMolecularData, run_pyscf

geometry = [('Li', (0., 0., 0.)), ('H', (0., 0., 1.4))]
basis = 'sto-3g'
multiplicity = 1
charge = 0

molecule = run_pyscf(
        PyscfMolecularData(geometry, basis, multiplicity, charge)
)

Output:

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-1-bf13348427ff> in <module>()
      7 
      8 molecule = run_pyscf(
----> 9         PyscfMolecularData(geometry, basis, multiplicity, charge)
     10 )

~/Projects/OpenFermion-PySCF/openfermionpyscf/_run_pyscf.py in run_pyscf(molecule, run_scf, run_mp2, run_cisd, run_ccsd, run_fci, verbose)
    143 
    144     # Populate fields.
--> 145     molecule.canonical_orbitals = pyscf_scf.mo_coeff.astype(float)
    146     molecule.orbital_energies = pyscf_scf.mo_energy.astype(float)
    147 

AttributeError: can't set attribute

The reason is that PyscfMolecularData has canonical_orbitals, etc., as properties rather than attributes. One could fix this by adding setters, but I don't know what the correct design or intended usage is. If we can calculate stuff on the fly, then perhaps we don't even need to perform run_pyscf on a PyscfMolecularData? @sunqm

sunqm commented 6 years ago

I like the design to compute orbitals and other quantities on the fly. At this stage, the overhead to recompute things is negligible. By the time when the interface was implemented, molecule.canonical_orbitals was a regular attribute than the property. To make the interface compatible with the old implementation, run_pyscf is kept and attributes like molecule.canonical_orbitals, molecule.orbital_energies are initialized in run_pyscf.

When computing things on the fly, the symmetry of the system needs to be carefully considered. The orbitals may be differed by a phase from run to run. If orbitals are recomputed, the MO integrals may be varied during the simulation accordingly, which may cause numerical instability.

snow0369 commented 1 month ago

I think this issue still persists. Do you have any ideas on how to resolve this problem? As a temporary solution, I wrote setters for all problematic properties, as shown below, but I am not familiar with the design of this project and am not sure if this is okay. Since @sunqm mentioned that recomputing may cause instability, the setter only updates when the values are not specified.

@canonical_orbitals.setter
def canonical_orbitals(self, value):
    if self._canonical_orbitals is None:
        self._canonical_orbitals = value
    else:
        raise AttributeError