From b0f948f2d659b8525db07339e309132ead1b6ebd Mon Sep 17 00:00:00 2001 From: Marvin Friede <51965259+marvinfriede@users.noreply.github.com> Date: Wed, 14 Aug 2024 10:20:58 -0500 Subject: [PATCH] Improve code quality (#157) --- .github/workflows/ubuntu-pytorch-1.yaml | 100 ++++++++++++++++++ .github/workflows/ubuntu.yaml | 18 +--- examples/profiling/dispersion.py | 11 +- setup.cfg | 2 +- src/dxtb/_src/basis/bas.py | 5 +- .../properties/vibration/analysis.py | 4 +- src/dxtb/_src/calculators/types/analytical.py | 12 +-- src/dxtb/_src/calculators/types/energy.py | 3 - src/dxtb/_src/cli/driver.py | 4 +- .../components/interactions/solvation/alpb.py | 5 +- .../integral/driver/libcint/base_driver.py | 2 + .../driver/pytorch/impls/md/explicit.py | 20 ++-- .../driver/pytorch/impls/md/recursion.py | 2 +- .../integral/driver/pytorch/impls/md/trafo.py | 20 ++-- src/dxtb/_src/integral/wrappers.py | 2 + src/dxtb/_src/io/handler.py | 5 + src/dxtb/_src/wavefunction/filling.py | 1 - src/dxtb/_src/xtb/gfn1.py | 1 - src/dxtb/integrals/wrappers.py | 2 + test/conftest.py | 9 ++ test/test_external/test_field.py | 3 +- test/test_hamiltonian/test_general.py | 3 +- test/test_mol/test_external.py | 2 +- test/test_repulsion/test_grad_pos.py | 2 - test/test_singlepoint/test_energy.py | 23 ++-- test/test_singlepoint/test_general.py | 14 +-- test/test_singlepoint/test_grad.py | 31 ++---- test/test_singlepoint/test_hess.py | 9 +- test/utils.py | 2 +- 29 files changed, 190 insertions(+), 127 deletions(-) create mode 100644 .github/workflows/ubuntu-pytorch-1.yaml diff --git a/.github/workflows/ubuntu-pytorch-1.yaml b/.github/workflows/ubuntu-pytorch-1.yaml new file mode 100644 index 00000000..28d68ff4 --- /dev/null +++ b/.github/workflows/ubuntu-pytorch-1.yaml @@ -0,0 +1,100 @@ +# This file is part of dxtb. +# +# SPDX-Identifier: Apache-2.0 +# Copyright (C) 2024 Grimme Group +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +name: Tests (Ubuntu, PyTorch V1) + +on: + push: + branches: + - main + - master + paths-ignore: + - "doc*/**" + - "./*.ya?ml" + - "**/*.md" + - "**/*.rst" + + pull_request: + paths-ignore: + - "doc*/**" + - "./*.ya?ml" + - "**/*.md" + - "**/*.rst" + + workflow_dispatch: + +jobs: + main: + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest] + python-version: ["3.8", "3.9", "3.10", "3.11"] + torch-version: ["1.11.0", "1.12.1", "1.13.1"] + exclude: + # Check latest versions here: https://download.pytorch.org/whl/torch/ + # + # PyTorch now fully supports Python=<3.11 + # see: https://github.com/pytorch/pytorch/issues/86566 + # + # PyTorch does now support Python 3.12 (Linux) for 2.2.0 and newer + # see: https://github.com/pytorch/pytorch/issues/110436 + # + # PyTorch<1.13.0 does only support Python=<3.10 + - python-version: "3.11" + torch-version: "1.11.0" + - python-version: "3.11" + torch-version: "1.12.1" + + runs-on: ${{ matrix.os }} + + defaults: + run: + shell: bash {0} + + steps: + - name: Checkout code + uses: actions/checkout@v3 + + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + + - name: Install dependencies + run: | + python3 -m pip install --upgrade pip + python3 -m pip install tox + + - name: Determine TOXENV + run: echo "TOXENV=py$(echo ${{ matrix.python-version }} | tr -d '.')-torch$(echo ${{ matrix.torch-version }} | tr -d '.')" >> $GITHUB_ENV + + - name: Print TOXENV + run: echo "TOXENV is set to '${{ env.TOXENV }}'." + + - name: Unittests with tox + run: tox -e ${{ env.TOXENV }} + + - name: Upload coverage to Codecov + uses: codecov/codecov-action@v3 + if: > + matrix.python-version == '3.11' && + matrix.torch-version == '2.2.2' && + matrix.os == 'ubuntu-latest' + with: + files: ./coverage.xml # optional + token: ${{ secrets.CODECOV_TOKEN }} # required + verbose: true # optional (default = false) diff --git a/.github/workflows/ubuntu.yaml b/.github/workflows/ubuntu.yaml index 1d20c9b8..9a8be185 100644 --- a/.github/workflows/ubuntu.yaml +++ b/.github/workflows/ubuntu.yaml @@ -43,32 +43,20 @@ jobs: matrix: os: [ubuntu-latest] # python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"] - # torch-version: ["1.11.0", "1.12.1", "1.13.1", "2.0.1", "2.1.2", "2.2.2", "2.3.1"] python-version: ["3.8", "3.9", "3.10", "3.11"] - torch-version: ["1.11.0", "1.12.1", "1.13.1", "2.0.1", "2.1.2", "2.2.2"] + torch-version: ["2.0.1", "2.1.2", "2.2.2", "2.3.1"] exclude: # Check latest versions here: https://download.pytorch.org/whl/torch/ # - # PyTorch now fully supports Python=<3.11 + # PyTorch fully supports Python=<3.11 # see: https://github.com/pytorch/pytorch/issues/86566 # - # PyTorch does now support Python 3.12 (Linux) for 2.2.0 and newer + # PyTorch supports Python 3.12 (Linux) for 2.2.0 and newer # see: https://github.com/pytorch/pytorch/issues/110436 - - python-version: "3.12" - torch-version: "1.11.0" - - python-version: "3.12" - torch-version: "1.12.1" - - python-version: "3.12" - torch-version: "1.13.1" - python-version: "3.12" torch-version: "2.0.1" - python-version: "3.12" torch-version: "2.1.2" - # PyTorch<1.13.0 does only support Python=<3.10 - - python-version: "3.11" - torch-version: "1.11.0" - - python-version: "3.11" - torch-version: "1.12.1" runs-on: ${{ matrix.os }} diff --git a/examples/profiling/dispersion.py b/examples/profiling/dispersion.py index 715e8d66..3a891419 100644 --- a/examples/profiling/dispersion.py +++ b/examples/profiling/dispersion.py @@ -44,7 +44,9 @@ dxtb.timer.start("Setup") dxtb.timer.start("Ihelp", parent_uid="Setup") -ihelp = dxtb.IndexHelper.from_numbers(numbers, dxtb.GFN1_XTB, batch_mode=batch_mode) +ihelp_cuda = dxtb.IndexHelper.from_numbers( + numbers, dxtb.GFN1_XTB, batch_mode=batch_mode +) dxtb.timer.stop("Ihelp") dxtb.timer.start("Class", parent_uid="Setup") @@ -56,7 +58,7 @@ dxtb.timer.start("Cache") torch.cuda.synchronize() -cache = obj.get_cache(numbers, ihelp=ihelp) +cache = obj.get_cache(numbers, ihelp=ihelp_cuda) torch.cuda.synchronize() dxtb.timer.stop("Cache") @@ -74,14 +76,13 @@ numbers = numbers.cpu() positions = positions.cpu() -ihelp = ihelp.cpu() dd: DD = {"device": torch.device("cpu"), "dtype": torch.double} dxtb.timer.reset() dxtb.timer.start("Setup") dxtb.timer.start("Ihelp", parent_uid="Setup") -ihelp = dxtb.IndexHelper.from_numbers(numbers, dxtb.GFN1_XTB, batch_mode=batch_mode) +ihelp_cpu = dxtb.IndexHelper.from_numbers(numbers, dxtb.GFN1_XTB, batch_mode=batch_mode) dxtb.timer.stop("Ihelp") dxtb.timer.start("Class", parent_uid="Setup") @@ -92,7 +93,7 @@ dxtb.timer.stop("Setup") dxtb.timer.start("Cache") -cache = obj.get_cache(numbers, ihelp=ihelp) +cache = obj.get_cache(numbers, ihelp=ihelp_cpu) dxtb.timer.stop("Cache") dxtb.timer.start("Energy") diff --git a/setup.cfg b/setup.cfg index 5bd3650a..d5dd850c 100644 --- a/setup.cfg +++ b/setup.cfg @@ -48,7 +48,7 @@ install_requires = tad-multicharge tomli tomli-w - torch>=1.11.0,<=2.2.2 + torch>=1.11.0,<2.4 typing-extensions python_requires = >=3.8, <3.12 package_dir = diff --git a/src/dxtb/_src/basis/bas.py b/src/dxtb/_src/basis/bas.py index 89a516d9..3b86267f 100644 --- a/src/dxtb/_src/basis/bas.py +++ b/src/dxtb/_src/basis/bas.py @@ -297,6 +297,7 @@ def to_bse( f"Available options are: {allowed_formats}." ) + header = "" if with_header is True: l = 70 * "-" header = ( @@ -317,9 +318,7 @@ def to_bse( s = 0 fulltxt = "" for i, number in enumerate(self.unique.tolist()): - txt = "" - if with_header is True: - txt += header # type: ignore + txt = header if qcformat == "gaussian94": txt += f"{pse.Z2S[number]}\n" diff --git a/src/dxtb/_src/calculators/properties/vibration/analysis.py b/src/dxtb/_src/calculators/properties/vibration/analysis.py index 56c0b8d9..b66eee47 100644 --- a/src/dxtb/_src/calculators/properties/vibration/analysis.py +++ b/src/dxtb/_src/calculators/properties/vibration/analysis.py @@ -140,10 +140,10 @@ def _get_rotational_modes(mass: Tensor, positions: Tensor): # Eigendecomposition yields the principal moments of inertia (w) # and the principal axes of rotation (paxes) of a molecule. - w, paxes = storch.eighb(im) + _, paxes = storch.eighb(im) # make z-axis rotation vector with smallest moment of inertia - w = torch.flip(w, [-1]) + # w = torch.flip(w, [-1]) paxes = torch.flip(paxes, [-1]) ex, ey, ez = paxes.mT diff --git a/src/dxtb/_src/calculators/types/analytical.py b/src/dxtb/_src/calculators/types/analytical.py index 93844d96..10882a60 100644 --- a/src/dxtb/_src/calculators/types/analytical.py +++ b/src/dxtb/_src/calculators/types/analytical.py @@ -99,6 +99,8 @@ def forces_analytical( Tensor Atomic forces of shape ``(..., nat, 3)``. """ + total_grad = torch.zeros(positions.shape, **self.dd) + # DEVNOTE: We need to save certain properties from the energy # calculation for the analytical derivative. So, we check in the # options if those quantities were cached. If not, we correct the @@ -111,14 +113,6 @@ def forces_analytical( self.energy(positions, chrg, spin, **kwargs) - # Setup - - chrg = any_to_tensor(chrg, **self.dd) - if spin is not None: - spin = any_to_tensor(spin, **self.dd) - - total_grad = torch.zeros(positions.shape, **self.dd) - # CLASSICAL CONTRIBUTIONS if len(self.classicals.components) > 0: @@ -139,7 +133,7 @@ def forces_analytical( timer.stop("Classicals Gradient") OutputHandler.write_stdout("done", v=3) - if any(x in ["all", "scf"] for x in self.opts.exclude): + if {"all", "scf"} & set(self.opts.exclude): return -total_grad # SELF-CONSISTENT FIELD PROCEDURE diff --git a/src/dxtb/_src/calculators/types/energy.py b/src/dxtb/_src/calculators/types/energy.py index c1f0f84e..dba8e07b 100644 --- a/src/dxtb/_src/calculators/types/energy.py +++ b/src/dxtb/_src/calculators/types/energy.py @@ -46,9 +46,6 @@ __all__ = ["EnergyCalculator"] -logger = logging.getLogger(__name__) - - class EnergyCalculator(BaseCalculator): """ Parametrized calculator defining the extended tight-binding model. diff --git a/src/dxtb/_src/cli/driver.py b/src/dxtb/_src/cli/driver.py index 7276dd50..f7e5e470 100644 --- a/src/dxtb/_src/cli/driver.py +++ b/src/dxtb/_src/cli/driver.py @@ -176,9 +176,7 @@ def singlepoint(self) -> Result | None: numbers = pack(_n) positions = pack(_p) else: - _n, _p = read.read_from_path(args.file[0], args.filetype) - numbers = torch.tensor(_n, dtype=torch.long, device=dd["device"]) - positions = torch.tensor(_p, **dd) + numbers, positions = read.read_from_path(args.file[0], args.filetype, **dd) timer.stop("Read Files") diff --git a/src/dxtb/_src/components/interactions/solvation/alpb.py b/src/dxtb/_src/components/interactions/solvation/alpb.py index 716fc635..f92e4589 100644 --- a/src/dxtb/_src/components/interactions/solvation/alpb.py +++ b/src/dxtb/_src/components/interactions/solvation/alpb.py @@ -64,7 +64,7 @@ from tad_mctc.math import einsum from dxtb._src.param import Param -from dxtb._src.typing import DD, Any, Tensor, TensorLike, get_default_dtype +from dxtb._src.typing import DD, Any, Tensor, TensorLike, get_default_dtype, override from dxtb._src.typing.exceptions import DeviceError from ..base import Interaction, InteractionCache @@ -234,6 +234,7 @@ def __init__( kwargs["rvdw"] = VDW_D3.to(**self.dd)[numbers] self.born_kwargs = kwargs + @override def get_cache( self, numbers: Tensor, positions: Tensor, **_ ) -> GeneralizedBornCache: @@ -289,9 +290,11 @@ def get_cache( self.cache = GeneralizedBornCache(mat) return self.cache + @override def get_atom_energy(self, charges: Tensor, cache: GeneralizedBornCache) -> Tensor: return 0.5 * charges * self.get_atom_potential(charges, cache) + @override def get_atom_potential( self, charges: Tensor, cache: GeneralizedBornCache ) -> Tensor: diff --git a/src/dxtb/_src/integral/driver/libcint/base_driver.py b/src/dxtb/_src/integral/driver/libcint/base_driver.py index e6fbd5c5..a4bcdfb4 100644 --- a/src/dxtb/_src/integral/driver/libcint/base_driver.py +++ b/src/dxtb/_src/integral/driver/libcint/base_driver.py @@ -110,6 +110,8 @@ def setup(self, positions: Tensor, **kwargs) -> None: IndexHelper.from_numbers(number, self.par) for number in self.numbers ] + else: + raise ValueError(f"Unknown batch mode '{self.ihelp.batch_mode}'.") assert isinstance(atombases, list) self.drv = [ diff --git a/src/dxtb/_src/integral/driver/pytorch/impls/md/explicit.py b/src/dxtb/_src/integral/driver/pytorch/impls/md/explicit.py index 01183ff0..e3658587 100644 --- a/src/dxtb/_src/integral/driver/pytorch/impls/md/explicit.py +++ b/src/dxtb/_src/integral/driver/pytorch/impls/md/explicit.py @@ -513,8 +513,6 @@ def de_s( e030 = rpj * e020 + e021 e020 = rpj * e010 + e011 - e021 = xij * e010 + rpj * e011 - e030 = rpj * e020 + e021 e022 = xij * e011 e031 = xij * e020 + rpj * e021 + 2 * e022 e040 = rpj * e030 + e031 @@ -789,14 +787,14 @@ def de_p( e210 = rpi * e110 + e111 f110 = a * e210 - e010 - e021 = xij * e010 + rpj * e011 - e030 = rpj * e020 + e021 + # e021 = xij * e010 + rpj * e011 + # e030 = rpj * e020 + e021 f020 = 2 * e010 - b * e030 - e111 = xij * e100 + rpj * e101 + # e111 = xij * e100 + rpj * e101 e112 = xij * e011 e211 = xij * e110 + rpi * e111 + 2 * e112 - e120 = rpj * e110 + e111 + # e120 = rpj * e110 + e111 e220 = rpj * e210 + e211 f120 = a * e220 - e020 @@ -1612,10 +1610,10 @@ def de_f( f110 = a * e210 - e010 f210 = a * e310 - 2 * e110 - e302 = xij * e201 + rpi * e202 - e021 = xij * e010 + rpj * e011 - e022 = xij * e011 - e032 = xij * e021 + rpj * e022 + # e302 = xij * e201 + rpi * e202 + # e021 = xij * e010 + rpj * e011 + # e022 = xij * e011 + # e032 = xij * e021 + rpj * e022 e311 = xij * e300 + rpj * e301 + 2 * e302 e410 = rpi * e310 + e311 @@ -1630,7 +1628,7 @@ def de_f( f120 = a * e220 - e020 f130 = a * e230 - e030 - e320 = rpj * e310 + e311 + # e320 = rpj * e310 + e311 f220 = a * e320 - 2 * e120 f230 = a * e330 - 2 * e130 diff --git a/src/dxtb/_src/integral/driver/pytorch/impls/md/recursion.py b/src/dxtb/_src/integral/driver/pytorch/impls/md/recursion.py index 76f756c3..2006b358 100644 --- a/src/dxtb/_src/integral/driver/pytorch/impls/md/recursion.py +++ b/src/dxtb/_src/integral/driver/pytorch/impls/md/recursion.py @@ -477,7 +477,7 @@ def md_recursion_gradient( # for single gaussians (e.g. in tests) if len(vec.shape) == 1: vec = torch.unsqueeze(vec, 0) - s3d = torch.unsqueeze(s3d, 0) + # s3d = torch.unsqueeze(s3d, 0) ds3d = torch.unsqueeze(ds3d, 0) # calc E function for all (ai, aj)-combis for all vecs in batch diff --git a/src/dxtb/_src/integral/driver/pytorch/impls/md/trafo.py b/src/dxtb/_src/integral/driver/pytorch/impls/md/trafo.py index c1b00b24..c7c2593b 100644 --- a/src/dxtb/_src/integral/driver/pytorch/impls/md/trafo.py +++ b/src/dxtb/_src/integral/driver/pytorch/impls/md/trafo.py @@ -47,17 +47,17 @@ s45 = sqrt(45.0) s45_8 = sqrt(45.0 / 8.0) -d38 = 3.0 / 8.0 -d34 = 3.0 / 4.0 -s5_16 = sqrt(5.0 / 16.0) +# d38 = 3.0 / 8.0 +# d34 = 3.0 / 4.0 +# s5_16 = sqrt(5.0 / 16.0) s10 = sqrt(10.0) -s10_8 = sqrt(10.0 / 8.0) -s35_4 = sqrt(35.0 / 4.0) -s35_8 = sqrt(35.0 / 8.0) -s35_64 = sqrt(35.0 / 64.0) -s45_4 = sqrt(45.0 / 4.0) -s315_8 = sqrt(315.0 / 8.0) -s315_16 = sqrt(315.0 / 16.0) +# s10_8 = sqrt(10.0 / 8.0) +# s35_4 = sqrt(35.0 / 4.0) +# s35_8 = sqrt(35.0 / 8.0) +# s35_64 = sqrt(35.0 / 64.0) +# s45_4 = sqrt(45.0 / 4.0) +# s315_8 = sqrt(315.0 / 8.0) +# s315_16 = sqrt(315.0 / 16.0) TRAFO = ( diff --git a/src/dxtb/_src/integral/wrappers.py b/src/dxtb/_src/integral/wrappers.py index 98393b77..fc72bbbe 100644 --- a/src/dxtb/_src/integral/wrappers.py +++ b/src/dxtb/_src/integral/wrappers.py @@ -256,6 +256,8 @@ def _integral( integral = Dipole(driver=driver_name, **dd, **kwargs) elif integral_type == "_quadrupole": integral = Quadrupole(driver=driver_name, **dd, **kwargs) + else: + raise ValueError(f"Unknown integral type '{integral_type}'.") integral.integral.norm = ovlp.integral.norm else: diff --git a/src/dxtb/_src/io/handler.py b/src/dxtb/_src/io/handler.py index fe0afe73..5031fc73 100644 --- a/src/dxtb/_src/io/handler.py +++ b/src/dxtb/_src/io/handler.py @@ -393,6 +393,11 @@ def write_table( main_format += "{:<27} {:>20}" sub_format = " {:<26} \033[37m{:>20}\033[0m" sub_format_no_indent = "{:<27} \033[37m{:>20}\033[0m" + else: + raise ValueError( + "Length of columns for printing a table must be either 2 or" + f"3 but got {len(columns)}." + ) if self.verbosity >= (v + 1): main_format += "\033[0m" diff --git a/src/dxtb/_src/wavefunction/filling.py b/src/dxtb/_src/wavefunction/filling.py index fd43259e..045216c9 100644 --- a/src/dxtb/_src/wavefunction/filling.py +++ b/src/dxtb/_src/wavefunction/filling.py @@ -321,7 +321,6 @@ def get_fermi_occupation( # no valence electrons if (torch.abs(nel.sum(-1)) < eps).any(): return torch.zeros_like(emo) - raise ValueError("Number of valence electrons cannot be zero.") if thr is None: thr = defaults.FERMI_THRESH diff --git a/src/dxtb/_src/xtb/gfn1.py b/src/dxtb/_src/xtb/gfn1.py index 4cbc566d..d1485909 100644 --- a/src/dxtb/_src/xtb/gfn1.py +++ b/src/dxtb/_src/xtb/gfn1.py @@ -425,7 +425,6 @@ def get_gradient( ) zero = torch.tensor(0.0, **self.dd) - eps = torch.tensor(torch.finfo(self.dtype).eps, **self.dd) # -------------------- # Eq.28: X(EN_A, EN_B) diff --git a/src/dxtb/integrals/wrappers.py b/src/dxtb/integrals/wrappers.py index 5231c17e..0040eb69 100644 --- a/src/dxtb/integrals/wrappers.py +++ b/src/dxtb/integrals/wrappers.py @@ -25,3 +25,5 @@ from dxtb._src.integral.wrappers import hcore as hcore from dxtb._src.integral.wrappers import overlap as overlap from dxtb._src.integral.wrappers import quadint as quadint + +__all__ = ["dipint", "hcore", "overlap", "quadint"] diff --git a/test/conftest.py b/test/conftest.py index f9a936fb..8003132a 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -39,6 +39,15 @@ """Name of Device.""" +# A bug in PyTorch 2.3.0 and 2.3.1 somehow requires manual import of +# `torch._dynamo` to avoid errors with functorch in custom backward +# functions. See https://github.com/pytorch/pytorch/issues/128607. +from tad_mctc._version import __tversion__ + +if __tversion__ in ((2, 3, 0), (2, 3, 1)): + import torch._dynamo + + def pytest_addoption(parser: pytest.Parser) -> None: """Set up additional command line options.""" diff --git a/test/test_external/test_field.py b/test/test_external/test_field.py index fc688348..d7db0ad0 100644 --- a/test/test_external/test_field.py +++ b/test/test_external/test_field.py @@ -35,8 +35,7 @@ from ..conftest import DEVICE from .samples import samples -sample_list = ["MB16_43_01"] -sample_list = ["LiH", "SiH4"] +sample_list = ["LiH", "SiH4", "MB16_43_01"] opts = { "verbosity": 0, diff --git a/test/test_hamiltonian/test_general.py b/test/test_hamiltonian/test_general.py index 1d177347..3b8f90df 100644 --- a/test/test_hamiltonian/test_general.py +++ b/test/test_hamiltonian/test_general.py @@ -27,7 +27,6 @@ from dxtb import GFN1_XTB as par from dxtb import IndexHelper -from dxtb._src.xtb.base import BaseHamiltonian from dxtb._src.xtb.gfn1 import GFN1Hamiltonian @@ -100,6 +99,8 @@ def test_change_device(device_str: str) -> None: dev = torch.device("cpu") elif device_str == "cuda": dev = torch.device("cuda:0") + else: + assert False h0 = h0.to(dev) assert h0.device == dev diff --git a/test/test_mol/test_external.py b/test/test_mol/test_external.py index 1abd22b5..c241ccc6 100644 --- a/test/test_mol/test_external.py +++ b/test/test_mol/test_external.py @@ -61,7 +61,7 @@ def test_construction(dtype: torch.dtype, name: str) -> None: num2, pos2 = a2 assert num1 == num2 - assert pos2.dtype == pos2.dtype # type: ignore + assert pos1.dtype == pos2.dtype # type: ignore assert pytest.approx(pos1) == pos2 diff --git a/test/test_repulsion/test_grad_pos.py b/test/test_repulsion/test_grad_pos.py index f25fc366..abf77b75 100644 --- a/test/test_repulsion/test_grad_pos.py +++ b/test/test_repulsion/test_grad_pos.py @@ -180,8 +180,6 @@ def calc_numerical_gradient( for i in range(n_atoms): for j in range(3): - er, el = 0.0, 0.0 - positions[i, j] += step er = rep.get_energy(positions, cache) er = torch.sum(er, dim=-1) diff --git a/test/test_singlepoint/test_energy.py b/test/test_singlepoint/test_energy.py index 1e69983c..f1af1282 100644 --- a/test/test_singlepoint/test_energy.py +++ b/test/test_singlepoint/test_energy.py @@ -56,12 +56,8 @@ def test_single(dtype: torch.dtype, name: str, scf_mode: str) -> None: base = Path(Path(__file__).parent, "mols", name) - numbers, positions = read.read_from_path(Path(base, "coord")) - charge = read.read_chrg_from_path(Path(base, ".CHRG")) - - numbers = torch.tensor(numbers, dtype=torch.long, device=DEVICE) - positions = torch.tensor(positions, **dd) - charge = torch.tensor(charge, **dd) + numbers, positions = read.read_from_path(Path(base, "coord"), **dd) + charge = read.read_chrg_from_path(Path(base, ".CHRG"), **dd) ref = samples[name]["etot"].to(**dd) @@ -90,12 +86,8 @@ def test_single_large(dtype: torch.dtype, name: str, scf_mode: str) -> None: base = Path(Path(__file__).parent, "mols", name) - numbers, positions = read.read_from_path(Path(base, "coord")) - charge = read.read_chrg_from_path(Path(base, ".CHRG")) - - numbers = torch.tensor(numbers, dtype=torch.long, device=DEVICE) - positions = torch.tensor(positions, **dd) - charge = torch.tensor(charge, **dd) + numbers, positions = read.read_from_path(Path(base, "coord"), **dd) + charge = read.read_chrg_from_path(Path(base, ".CHRG"), **dd) ref = samples[name]["etot"].to(**dd) @@ -217,12 +209,9 @@ def test_uhf_single(dtype: torch.dtype, name: str) -> None: dd: DD = {"device": DEVICE, "dtype": dtype} base = Path(Path(__file__).parent, "mols", name) - numbers, positions = read.read_from_path(Path(base, "coord")) - charge = read.read_chrg_from_path(Path(base, ".CHRG")) + numbers, positions = read.read_from_path(Path(base, "coord"), **dd) + charge = read.read_chrg_from_path(Path(base, ".CHRG"), **dd) - numbers = torch.tensor(numbers, dtype=torch.long, device=DEVICE) - positions = torch.tensor(positions, **dd) - charge = torch.tensor(charge, **dd) ref = samples[name]["etot"].to(**dd) calc = Calculator(numbers, par, opts=opts, **dd) diff --git a/test/test_singlepoint/test_general.py b/test/test_singlepoint/test_general.py index b1094457..efdb170a 100644 --- a/test/test_singlepoint/test_general.py +++ b/test/test_singlepoint/test_general.py @@ -23,7 +23,6 @@ from pathlib import Path import pytest -import torch from tad_mctc.io import read from dxtb import GFN1_XTB as par @@ -35,21 +34,18 @@ opts = {"verbosity": 0, "int_level": 4} +@pytest.mark.filterwarnings("ignore::tad_mctc.exceptions.MoleculeWarning") def test_uhf_fail() -> None: - # singlepoint starts SCF timer, but exception is thrown before the SCF - # timer is stopped, so we must disable it here + # Singlepoint starts SCF timer, but exception is thrown before the SCF + # timer is stopped, so we must disable it here. status = timer._enabled if status is True: timer.disable() base = Path(Path(__file__).parent, "mols", "H") - numbers, positions = read.read_from_path(Path(base, "coord")) - charge = read.read_chrg_from_path(Path(base, ".CHRG")) - - numbers = torch.tensor(numbers, dtype=torch.long) - positions = torch.tensor(positions) - charge = torch.tensor(charge) + numbers, positions = read.read_from_path(Path(base, "coord"), device=DEVICE) + charge = read.read_chrg_from_path(Path(base, ".CHRG"), device=DEVICE) calc = Calculator(numbers, par, opts=opts, device=DEVICE) diff --git a/test/test_singlepoint/test_grad.py b/test/test_singlepoint/test_grad.py index b7f8fc25..b1c39e56 100644 --- a/test/test_singlepoint/test_grad.py +++ b/test/test_singlepoint/test_grad.py @@ -91,13 +91,10 @@ def analytical( # read from file base = Path(Path(__file__).parent, "mols", name) - numbers, positions = read.read_from_path(Path(base, "coord")) - charge = read.read_chrg_from_path(Path(base, ".CHRG")) + numbers, positions = read.read_from_path(Path(base, "coord"), **dd) + charge = read.read_chrg_from_path(Path(base, ".CHRG"), **dd) - # convert to tensors - numbers = torch.tensor(numbers, dtype=torch.long, device=DEVICE) - positions = torch.tensor(positions, **dd, requires_grad=True) - charge = torch.tensor(charge, **dd) + positions = positions.clone().requires_grad_(True) options = dict( opts, @@ -127,13 +124,10 @@ def test_backward(dtype: torch.dtype, name: str, scf_mode: str) -> None: # read from file base = Path(Path(__file__).parent, "mols", name) - numbers, positions = read.read_from_path(Path(base, "coord")) - charge = read.read_chrg_from_path(Path(base, ".CHRG")) + numbers, positions = read.read_from_path(Path(base, "coord"), **dd) + charge = read.read_chrg_from_path(Path(base, ".CHRG"), **dd) - # convert to tensors - numbers = torch.tensor(numbers, dtype=torch.long, device=DEVICE) - positions = torch.tensor(positions, **dd, requires_grad=True) - charge = torch.tensor(charge, **dd) + positions = positions.clone().requires_grad_(True) # do calc options = dict( @@ -173,22 +167,17 @@ def test_num(name: str, scf_mode: str) -> None: # read from file base = Path(Path(__file__).parent, "mols", name) - numbers, positions = read.read_from_path(Path(base, "coord")) - charge = read.read_chrg_from_path(Path(base, ".CHRG")) - - # convert to tensors - numbers = torch.tensor(numbers, dtype=torch.long, device=DEVICE) - positions = torch.tensor(positions, **dd) - charge = torch.tensor(charge, **dd) + numbers, positions = read.read_from_path(Path(base, "coord"), **dd) + charge = read.read_chrg_from_path(Path(base, ".CHRG"), **dd) # do calc - gradient = calc_numerical_gradient(numbers, positions, charge, scf_mode, dd) + gradient = num_grad(numbers, positions, charge, scf_mode, dd) ref = load_from_npz(ref_grad, name, dtype) assert pytest.approx(ref.cpu(), abs=1e-6, rel=1e-4) == gradient.cpu() -def calc_numerical_gradient( +def num_grad( numbers: Tensor, positions: Tensor, charge: Tensor, scf_mode: str, dd: DD ) -> Tensor: """Calculate gradient numerically for reference.""" diff --git a/test/test_singlepoint/test_hess.py b/test/test_singlepoint/test_hess.py index 08e23a24..8b213be7 100644 --- a/test/test_singlepoint/test_hess.py +++ b/test/test_singlepoint/test_hess.py @@ -59,13 +59,8 @@ def test_single(dtype: torch.dtype, name: str) -> None: # read from file base = Path(Path(__file__).parent, "mols", name) - numbers, positions = read.read_from_path(Path(base, "coord")) - charge = read.read_chrg_from_path(Path(base, ".CHRG")) - - # convert to tensors - numbers = torch.tensor(numbers, dtype=torch.long, device=DEVICE) - positions = torch.tensor(positions, **dd) - charge = torch.tensor(charge, **dd) + numbers, positions = read.read_from_path(Path(base, "coord"), **dd) + charge = read.read_chrg_from_path(Path(base, ".CHRG"), **dd) ref = reshape_fortran( ( diff --git a/test/utils.py b/test/utils.py index 174efd83..93921d43 100644 --- a/test/utils.py +++ b/test/utils.py @@ -24,7 +24,7 @@ import torch -from dxtb._src.typing import Any, Callable, Protocol, Size, Tensor, TensorOrTensors +from dxtb._src.typing import Any, Size, Tensor coordfile = Path(Path(__file__).parent, "test_singlepoint/mols/H2/coord").resolve() """Path to coord file of H2."""