Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

run_pyscf does not work on PyscfMolecularData #38

Open
kevinsung opened this issue Aug 25, 2018 · 2 comments
Open

run_pyscf does not work on PyscfMolecularData #38

kevinsung opened this issue Aug 25, 2018 · 2 comments

Comments

@kevinsung
Copy link
Collaborator

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
Copy link
Contributor

sunqm commented Aug 25, 2018

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants