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

Rename nprim to nexp where appropriate #357

Merged
merged 1 commit into from
Jul 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions iodata/basis.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,12 @@ class Shell:
"""

exponents: NDArray[float] = attrs.field(validator=validate_shape(("coeffs", 0)))
"""The array containing the exponents of the primitives, with shape (nprim,)."""
"""The array containing the exponents of the primitives, with shape (nexp,)."""

coeffs: NDArray[float] = attrs.field(validator=validate_shape(("exponents", 0), ("kinds", 0)))
"""
The array containing the coefficients of the normalized primitives in each contraction;
shape = (nprim, ncon).
shape = (nexp, ncon).
These coefficients assume that the primitives are L2 (orbitals) or L1
(densities) normalized, but contractions are not necessarily normalized.
(This depends on the code which generated the contractions.)
Expand All @@ -143,8 +143,8 @@ def nbasis(self) -> int:
return result

@property
def nprim(self) -> int:
"""Number of primitives, also known as the contraction length."""
def nexp(self) -> int:
"""Number of exponents in the contracted shell, also known as the contraction length."""
return len(self.exponents)

@property
Expand Down
20 changes: 10 additions & 10 deletions iodata/formats/fchk.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,15 +157,15 @@ def load_one(lit: LineIterator) -> dict:
# B) Load the orbital basis set
shell_types = fchk["Shell types"]
shell_map = fchk["Shell to atom map"] - 1
nprims = fchk["Number of primitives per shell"]
nexps = fchk["Number of primitives per shell"]
exponents = fchk["Primitive exponents"]
ccoeffs_level1 = fchk["Contraction coefficients"]
ccoeffs_level2 = fchk.get("P(S=P) Contraction coefficients")

shells = []
counter = 0
# First loop over all shells
for i, n in enumerate(nprims):
for i, n in enumerate(nexps):
if shell_types[i] == -1:
# Special treatment for SP shell type
shells.append(
Expand Down Expand Up @@ -196,7 +196,7 @@ def load_one(lit: LineIterator) -> dict:
counter += n
del shell_map
del shell_types
del nprims
del nexps
del exponents

result["obasis"] = MolecularBasis(shells, CONVENTIONS, "L2")
Expand Down Expand Up @@ -645,9 +645,9 @@ def dump_one(f: TextIO, data: IOData):
# write molecular orbital basis set
if data.obasis is not None:
# number of primitives per shell
nprims = np.array([shell.nprim for shell in data.obasis.shells])
nexps = np.array([shell.nexp for shell in data.obasis.shells])
exponents = np.array([item for shell in data.obasis.shells for item in shell.exponents])
coeffs = np.array([s.coeffs[i][0] for s in data.obasis.shells for i in range(s.nprim)])
coeffs = np.array([s.coeffs[i][0] for s in data.obasis.shells for i in range(s.nexp)])
coordinates = np.array([data.atcoords[shell.icenter] for shell in data.obasis.shells])
shell_to_atom = np.array([shell.icenter + 1 for shell in data.obasis.shells])

Expand All @@ -669,14 +669,14 @@ def dump_one(f: TextIO, data: IOData):
_dump_integer_scalars("Number of basis functions", data.obasis.nbasis, f)
_dump_integer_scalars("Number of independent functions", data.obasis.nbasis, f)
_dump_integer_scalars("Number of contracted shells", len(data.obasis.shells), f)
_dump_integer_scalars("Number of primitive shells", nprims.sum(), f)
_dump_integer_scalars("Number of primitive shells", nexps.sum(), f)
_dump_integer_scalars("Pure/Cartesian d shells", num_pure_d_shells, f)
_dump_integer_scalars("Pure/Cartesian f shells", num_pure_f_shells, f)
_dump_integer_scalars("Highest angular momentum", np.amax(np.abs(shell_types)), f)
_dump_integer_scalars("Largest degree of contraction", np.amax(nprims), f)
_dump_integer_scalars("Largest degree of contraction", np.amax(nexps), f)

_dump_integer_arrays("Shell types", np.array(shell_types), f)
_dump_integer_arrays("Number of primitives per shell", nprims, f)
_dump_integer_arrays("Number of primitives per shell", nexps, f)
_dump_integer_arrays("Shell to atom map", shell_to_atom, f)

_dump_real_arrays("Primitive exponents", exponents, f)
Expand All @@ -686,9 +686,9 @@ def dump_one(f: TextIO, data: IOData):
sp_coeffs = []
for shell, shell_type in zip(data.obasis.shells, shell_types):
if shell_type == -1:
sp_coeffs.extend([shell.coeffs[i][1] for i in range(shell.nprim)])
sp_coeffs.extend([shell.coeffs[i][1] for i in range(shell.nexp)])
else:
sp_coeffs.extend([0.0] * shell.nprim)
sp_coeffs.extend([0.0] * shell.nexp)
_dump_real_arrays("P(S=P) Contraction coefficients", np.array(sp_coeffs), f)
_dump_real_arrays("Coordinates of each shell", coordinates.flatten(), f)

Expand Down
12 changes: 6 additions & 6 deletions iodata/formats/molden.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,10 +276,10 @@ def _load_helper_obasis(lit: LineIterator) -> MolecularBasis:
break
# Read a new shell
angmom = angmom_sti(words[0])
nprim = int(words[1])
exponents = np.zeros(nprim)
coeffs = np.zeros((nprim, 1))
for iprim in range(nprim):
nexp = int(words[1])
exponents = np.zeros(nexp)
coeffs = np.zeros((nexp, 1))
for iprim in range(nexp):
words = next(lit).split()
exponents[iprim] = float(words[0].replace("D", "E"))
coeffs[iprim, 0] = float(words[1].replace("D", "E"))
Expand Down Expand Up @@ -528,7 +528,7 @@ def _fix_obasis_turbomole(obasis: MolecularBasis) -> Union[MolecularBasis, None]
fixed_shells.append(fixed_shell)
angmom = shell.angmoms[0]
kind = shell.kinds[0]
for iprim in range(shell.nprim):
for iprim in range(shell.nexp):
# Default 1.0: do not to correct anything, unless we know how to correct.
correction = 1.0
if angmom == 2 and kind == "c":
Expand Down Expand Up @@ -872,7 +872,7 @@ def dump_one(f: TextIO, data: IOData):
# Write out as a segmented basis. Molden format does not support
# generalized contractions.
for iangmom, angmom in enumerate(shell.angmoms):
f.write(f" {angmom_its(angmom):1s} {shell.nprim:3d} 1.00\n")
f.write(f" {angmom_its(angmom):1s} {shell.nexp:3d} 1.00\n")
for exponent, coeff in zip(shell.exponents, shell.coeffs[:, iangmom]):
f.write(f"{exponent:20.10f} {coeff:20.10f}\n")
f.write("\n")
Expand Down
4 changes: 2 additions & 2 deletions iodata/formats/mwfn.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,11 @@ def _load_helper_shells(lit: LineIterator, nshell: int) -> dict:


def _load_helper_section(
lit: LineIterator, nprim: int, start: str, skip: int, dtype: np.dtype
lit: LineIterator, size: int, start: str, skip: int, dtype: np.dtype
) -> NDArray:
"""Read single or multiple line(s) section."""
section = []
while len(section) < nprim:
while len(section) < size:
line = next(lit)
if not line.startswith(start):
raise LoadError(f"Expected line to start with {start}. Got line={line}.", lit)
Expand Down
2 changes: 1 addition & 1 deletion iodata/formats/wfn.py
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,7 @@ def dump_one(f: TextIO, data: IOData) -> None:
for angmom, kind in zip(shell.angmoms, shell.kinds):
n = len(data.obasis.conventions[angmom, kind])
c = raw_coeffs[index_mo_old : index_mo_old + n]
for _ in range(shell.nprim):
for _ in range(shell.nexp):
mo_coeffs[index_mo_new : index_mo_new + n] = c
index_mo_new += n
index_mo_old += n
Expand Down
2 changes: 1 addition & 1 deletion iodata/formats/wfx.py
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ def dump_one(f: TextIO, data: IOData):
for angmom, kind in zip(shell.angmoms, shell.kinds):
n = len(data.obasis.conventions[angmom, kind])
c = raw_coeffs[index_mo_old : index_mo_old + n]
for _j in range(shell.nprim):
for _j in range(shell.nexp):
mo_coeffs[index_mo_new : index_mo_new + n] = c
index_mo_new += n
index_mo_old += n
Expand Down
10 changes: 5 additions & 5 deletions iodata/test/test_basis.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,11 @@ def test_shell_info_propertes():
assert shells[2].nbasis == 4
assert shells[3].nbasis == 5
assert shells[4].nbasis == 6 + 7 + 9
assert shells[0].nprim == 6
assert shells[1].nprim == 3
assert shells[2].nprim == 1
assert shells[3].nprim == 2
assert shells[4].nprim == 1
assert shells[0].nexp == 6
assert shells[1].nexp == 3
assert shells[2].nexp == 1
assert shells[3].nexp == 2
assert shells[4].nexp == 1
Comment on lines +97 to +101
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Consider adding edge cases for nexp values

It would be beneficial to add tests for edge cases such as nexp being 0 or negative, even if these cases are not expected in normal operation. This ensures robustness against unexpected input.

Suggested change
assert shells[0].nexp == 6
assert shells[1].nexp == 3
assert shells[2].nexp == 1
assert shells[3].nexp == 2
assert shells[4].nexp == 1
assert shells[0].nexp == 6
assert shells[1].nexp == 3
assert shells[2].nexp == 1
assert shells[3].nexp == 2
assert shells[4].nexp == 1
assert all(shell.nexp >= 0 for shell in shells)

assert shells[0].ncon == 1
assert shells[1].ncon == 2
assert shells[2].ncon == 2
Expand Down
8 changes: 4 additions & 4 deletions iodata/test/test_fchk.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def test_load_fchk_hf_sto3g_num():
assert shell0.kinds == ["c"]
assert_allclose(shell0.exponents, np.array([1.66679134e02, 3.03608123e01, 8.21682067e00]))
assert_allclose(shell0.coeffs, np.array([[1.54328967e-01], [5.35328142e-01], [4.44634542e-01]]))
assert shell0.nprim == 3
assert shell0.nexp == 3
assert shell0.ncon == 1
assert shell0.nbasis == 1
shell1 = mol.obasis.shells[1]
Expand All @@ -87,11 +87,11 @@ def test_load_fchk_hf_sto3g_num():
]
),
)
assert shell1.nprim == 3
assert shell1.nexp == 3
assert shell1.ncon == 2
assert shell1.nbasis == 4
shell2 = mol.obasis.shells[2]
assert shell2.nprim == 3
assert shell2.nexp == 3
assert shell2.ncon == 1
assert shell2.nbasis == 1
assert mol.obasis.primitive_normalization == "L2"
Expand Down Expand Up @@ -123,7 +123,7 @@ def test_load_fchk_h_sto3g_num():
assert mol.title == "h_sto3g"
assert len(mol.obasis.shells) == 1
assert mol.obasis.nbasis == 1
assert mol.obasis.shells[0].nprim == 3
assert mol.obasis.shells[0].nexp == 3
Comment on lines 123 to +126
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Add tests for invalid nexp values

Consider adding tests to handle invalid nexp values, such as non-integer or negative values, to ensure the system behaves correctly under these conditions.

assert len(mol.atcoords) == len(mol.atnums)
assert mol.atcoords.shape[1] == 3
assert len(mol.atnums) == 1
Expand Down
6 changes: 3 additions & 3 deletions iodata/test/test_molden.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,19 +154,19 @@ def test_load_molden_low_nh3_molden_cart():
assert shell.icenter == 3

shell0 = obasis.shells[0]
assert shell0.nprim == 8
assert shell0.nexp == 8
assert shell0.exponents.shape == (8,)
assert_allclose(shell0.exponents[4], 0.2856000000e02)
assert shell0.coeffs.shape == (8, 1)
assert_allclose(shell0.coeffs[4, 0], 0.2785706633e00)
shell7 = obasis.shells[7]
assert shell7.nprim == 1
assert shell7.nexp == 1
assert shell7.exponents.shape == (1,)
assert_allclose(shell7.exponents, [0.8170000000e00])
assert_allclose(shell7.coeffs, [[1.0]])
assert shell7.coeffs.shape == (1, 1)
shell19 = obasis.shells[19]
assert shell19.nprim == 3
assert shell19.nexp == 3
assert shell19.exponents.shape == (3,)
assert_allclose(shell19.exponents, [0.1301000000e02, 0.1962000000e01, 0.4446000000e00])
assert_allclose(shell19.coeffs, [[0.3349872639e-01], [0.2348008012e00], [0.8136829579e00]])
Expand Down
2 changes: 1 addition & 1 deletion iodata/test/test_molekel.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ def test_load_mkl_ethanol():
# assert_allclose(mol.obasis.shells[-1].coeffs[-1, -1], 0.181380684)
assert_equal([shell.icenter for shell in mol.obasis.shells[:5]], [0, 0, 1, 1, 1])
assert_equal([shell.angmoms[0] for shell in mol.obasis.shells[:5]], [0, 0, 0, 0, 1])
assert_equal([shell.nprim for shell in mol.obasis.shells[:5]], [3, 1, 6, 3, 3])
assert_equal([shell.nexp for shell in mol.obasis.shells[:5]], [3, 1, 6, 3, 3])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Include tests for invalid nexp values

It would be beneficial to include tests for invalid nexp values, such as non-integer or negative values, to ensure the system handles these gracefully.

Suggested change
assert_equal([shell.nexp for shell in mol.obasis.shells[:5]], [3, 1, 6, 3, 3])
assert_equal([shell.nexp for shell in mol.obasis.shells[:5]], [3, 1, 6, 3, 3])
assert_equal(mol.mo.coeffs.shape, (39, 39))
assert_equal(mol.mo.energies.shape, (39,))
assert_equal(mol.mo.occs.shape, (39,))
with pytest.raises(ValueError):
mol.obasis.shells[0].nexp = -1
with pytest.raises(TypeError):
mol.obasis.shells[0].nexp = "invalid"

assert_equal(mol.mo.coeffs.shape, (39, 39))
assert_equal(mol.mo.energies.shape, (39,))
assert_equal(mol.mo.occs.shape, (39,))
Expand Down
6 changes: 3 additions & 3 deletions iodata/test/test_mwfn.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,16 @@ def test_load_mwfn_ch3_rohf_g03():
assert_equal(mol.mo.occs.max(), 2.0)
assert_equal(mol.extra["full_virial_ratio"], 2.00174844)
assert_equal(mol.extra["nindbasis"], 8)
assert_equal(np.sum([shell.nprim * shell.nbasis for shell in mol.obasis.shells]), 24)
assert_equal(np.sum([shell.nexp * shell.nbasis for shell in mol.obasis.shells]), 24)
assert_equal(len(mol.obasis.shells), 6)
assert_equal(np.sum([shell.nprim for shell in mol.obasis.shells]), 18)
assert_equal(np.sum([shell.nexp for shell in mol.obasis.shells]), 18)
assert_equal(mol.charge, 0.0)
assert_equal(mol.nelec, 9)
assert_equal(mol.natom, 4)
assert_equal(mol.energy, -3.90732095e01)
assert_allclose([shell.angmoms[0] for shell in mol.obasis.shells], [0, 0, 1, 0, 0, 0])
assert_allclose([shell.icenter for shell in mol.obasis.shells], [0, 0, 0, 1, 2, 3])
assert_allclose([shell.nprim for shell in mol.obasis.shells], [3, 3, 3, 3, 3, 3])
assert_allclose([shell.nexp for shell in mol.obasis.shells], [3, 3, 3, 3, 3, 3])
Comment on lines +43 to +52
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Add tests for boundary values of nexp

Consider adding tests for boundary values of nexp, such as 0 or very large numbers, to ensure the system can handle these cases without issues.

exponents1 = np.array([7.16168373e01, 1.30450963e01, 3.53051216e00])
exponents2 = np.array([2.94124936e00, 6.83483096e-01, 2.22289916e-01])
exponents3 = np.array([2.94124936e00, 6.83483096e-01, 2.22289916e-01])
Expand Down