oimeitei / quemb

QuEmb is an open-source tool for efficient quantum chemistry simulation of large molecules and solids (1D and 2D periodic systems) via bootstrap embedding technique.
Apache License 2.0
4 stars 6 forks source link

Linting refactoring/3 manual fixes flagged by ruff #57

Closed mcocdawc closed 1 week ago

mcocdawc commented 2 weeks ago

Only merge after the current test-suite is fixed #53 Adresses one task in #54 merge after #55 and #56

made manual fixes of which I am sure and ignore E741

mcocdawc commented 2 weeks ago

Just to give some exemplary easily avoided bugs that were found by the linting:

eritransform_parallel

    from molbe.external.eri_transform import get_emb_eri_fast_gdf

in kbe.pbe.eritransform_parallel would fail, because the submodule does not even exists. Moving all imports to the top and using libdmet.basis_transform.eri_transform.get_emb_eri_fast_gdf throughout fixes it.

examples.kbe_polyacetylene

# Define fragment in the supercell
kfrag = fragpart(be_type="be2", mol=cell, kpt=kpt, frozen_core=True)
# Initialize BE
mykbe = BE(kmf, fobj, kpts=kpts)

# here the fobj is undefined and should be kfrag

examples.molbe_pp.py

mf = scf.RHF(mol)
mf.conv_tol = 1e-12
mf.kernel()

# Define fragments; use IAO scheme with 'sto-3g' as the minimal basis set
fobj = fragpart(be_type="be2", mol=mol, valence_basis="sto-3g", frozen_core=True)

# Initialize BE
mybe = BE(mg, fobj, lo_method="iao")

# here the mg is not defined and should be mf
mcocdawc commented 2 weeks ago

Examples for constructs that raised a warning, but were not yet fixed because they require a deeper inspection and background knowlege:

molbe.ube

In the following code the dm_init is never used.

dm_init = fobj_b.get_nsocc(self.S, self.C_b, self.Nocc[1], ncore=self.ncore)

Since get_nsocc has unfortunately side-effects, it requires more careful inspection if the line can be removed or replaced with:

fobj_b.get_nsocc(self.S, self.C_b, self.Nocc[1], ncore=self.ncore)

i.e. call get_nsocc purely for its side-effect.

kbe.lo

There is a line

ovlp_ciao = uciao[k].conj().T @ self.S[k] @ Ciao[k]

The name uciao is undefined, most likely it should be

ovlp_ciao = Ciao[k].conj().T @ self.S[k] @ Ciao[k]

but one has to make sure this was actually intended.

kbe.pbe

The variable ECOUL is unused and initialized to 0.0

        ECOUL = 0.0

lateron in the code a variable ecoul is used. Most likely this was a typo and mismatched lower- vs upper-case.

molbe.external.jac_utils

In the function get_dt1ao_an the variable deov is assigned to but never used.

Since someone actually chose a name for it

deov = get_Dia_r(dmoe, no)

it might actually be relevant for later expressions and was just forgotten.

mcocdawc commented 1 week ago

PR is postponed