Run utils enhancements

.github/workflows/sphinx-docs.yaml

@@ -48,6 +48,7 @@ jobs:
           path: docs/build/html
 
   deploy:
+    if: github.event_name == 'push' && github.ref == 'refs/heads/main'
     environment:
       name: github-pages
       url: ${{ steps.deployment.outputs.page_url }}

README.md

@@ -60,10 +60,10 @@ It is also possible to generate `aims.out` files using an FHI-aims binary, which
 To do this, call
 
 ``` shell
-pytest -s --run_aims
+pytest -s --run-aims
 ```
 
-This will automatically prompt the user for the location of the FHI-aims binary using `$EDITOR`. Then, it will create a copy of the aims fixture directory but without the `aims.out` files, and run them using the custom FHI-aims binary. The tests will then run on these new generated files. As `dfttoolkit` stores the FHI-aims binary location so the user isn't prompted for its location every time `pytest` is run like this, `--run_aims` takes an optional argument `change_bin`, which will prompt the user again for the binary location and re-run the custom tests, regardless of whether they have been run before or not. 
+This will automatically prompt the user for the location of the FHI-aims binary using `$EDITOR`. Then, it will create a copy of the aims fixture directory but without the `aims.out` files, and run them using the custom FHI-aims binary. The tests will then run on these new generated files. As `dfttoolkit` stores the FHI-aims binary location so the user isn't prompted for its location every time `pytest` is run like this, `--run-aims` takes an optional argument `change_bin`, which will prompt the user again for the binary location and re-run the custom tests, regardless of whether they have been run before or not. 
 
 Currently, it is necessary to also run this with `-s` in order to capture the STDOUT to show the prompt. Also note that this will likely take some time to run this calculation, and it runs with 4 threads by default using `mpirun`. Finally, it is almost certain that several tests will fail, especially those relating to timing of calculations
 

dfttoolkit/output.py

@@ -967,9 +967,9 @@ def get_n_initial_ks_states(self, include_spin_polarised=True) -> int:
 
         Parameters
         ----------
-        include_spin_polarised : bool, optional
+        include_spin_polarised : bool, default=True
             Whether to include the spin-down states in the count if the calculation is
-            spin polarised (the default is True).
+            spin polarised.
 
         Returns
         -------

dfttoolkit/utils/math_utils.py

@@ -240,10 +240,10 @@ def get_autocorrelation_function_manual_lag(
     ----------
     signal : 1D npt.NDArray
         Siganl for which the autocorrelation function should be calculated.
-    max_lag : Union[None, int], optional
+    max_lag : Union[None, int]
         Autocorrelation will be calculated for a range of 0 to max_lag,
         where max_lag is the largest lag for the calculation of the
-        autocorrelation function. The default is None.
+        autocorrelation function
 
     Returns
     -------

dfttoolkit/utils/run_utils.py

@@ -1,40 +1,48 @@
 import os.path
-import warnings
 from functools import wraps
 
 
-def no_repeat(func):
+def no_repeat(
+    original_func=None,
+    *,
+    output_file: str = "aims.out",
+    calc_dir: str = "./",
+    force: bool = False,
+):
     """
     Don't repeat the calculation if aims.out exists in the calculation directory.
 
-    A kwarg must be given to the decorated function called `calc_dir` which is the
-    directory where the calculation is to be performed.
+    Parameters
+    ----------
+    output_file : str, default='aims.out'
+        The name of the output file to check for.
+    calc_dir : str, default="./"
+        The directory where the calculation is performed
+    force : bool, default=False
+        If True, the calculation is performed even if aims.out exists in the calculation
+        directory.
 
     Raises
     -------
     ValueError
         if the `calc_dir` kwarg is not a directory
     """
 
-    @wraps(func)
-    def wrapper_no_repeat(*args, **kwargs):
-        if "calc_dir" in kwargs and "force" in kwargs:
-            if not os.path.isdir(kwargs["calc_dir"]):
-                raise ValueError(f"{kwargs.get('calc_dir')} is not a directory.")
-
-            if kwargs["force"]:
+    def _no_repeat(func):
+        @wraps(func)
+        def wrapper(*args, **kwargs):
+            if not os.path.isdir(calc_dir):
+                raise ValueError(f"{calc_dir} is not a directory.")
+            if force:
                 return func(*args, **kwargs)
-            if not os.path.isfile(f"{kwargs.get('calc_dir')}/aims.out"):
+            if not os.path.isfile(f"{calc_dir}/{output_file}"):
                 return func(*args, **kwargs)
             else:
-                print(
-                    f"aims.out already exists in {kwargs.get('calc_dir')}. Skipping "
-                    "calculation."
-                )
+                print(f"aims.out already exists in {calc_dir}. Skipping calculation.")
+
+        return wrapper
 
-        else:
-            warnings.warn(
-                "'calc_dir' and/or 'force' kwarg not provided: ignoring decorator"
-            )
+    if original_func:
+        return _no_repeat(original_func)
 
-    return wrapper_no_repeat
+    return _no_repeat

dfttoolkit/utils/vibrations_utils.py

@@ -56,6 +56,7 @@ def get_cross_correlation_function(
     return cross_correlation
 
 
+# TODO Fix docstrings and types
 def get_cross_spectrum(
     signal_0: npt.NDArray,
     signal_1: npt.NDArray,
@@ -83,16 +84,16 @@ def get_cross_spectrum(
         Second siganl for which the correlation function should be calculated.
     time_step : float
         DESCRIPTION.
-    bootstrapping_blocks : int, optional
-        DESCRIPTION. The default is 1.
-    bootstrapping_overlap : int, optional
-        DESCRIPTION. The default is 0.
-    zero_padding : int, optional
+    bootstrapping_blocks : int, default=1
+        DESCRIPTION
+    bootstrapping_overlap : int, default=0
+        DESCRIPTION
+    zero_padding : int, default=0
         Pad the cross correlation function with zeros to increase the frequency
         resolution of the FFT. This also avoids the effect of varying spectral
         leakage. However, it artificially broadens the resulting cross spectrum
         and introduces wiggles.
-    cutoff_at_last_maximum : bool, optional
+    cutoff_at_last_maximum : bool, default=False
         Cut off the cross correlation function at the last maximum to hide
         spectral leakage.
 
@@ -114,7 +115,8 @@ def get_cross_spectrum(
     signal_length = len(signal_0)
     block_size = int(
         np.floor(
-            signal_length * (1 + bootstrapping_overlap)
+            signal_length
+            * (1 + bootstrapping_overlap)
             / (bootstrapping_blocks + bootstrapping_overlap)
         )
     )
@@ -123,8 +125,7 @@ def get_cross_spectrum(
     cross_spectrum = []
 
     for block in range(bootstrapping_blocks):
-        block_start = int(np.ceil(block * block_size /
-                          (1 + bootstrapping_overlap)))
+        block_start = int(np.ceil(block * block_size / (1 + bootstrapping_overlap)))
         if block_start < 0:
             block_start = 0
 
@@ -204,8 +205,7 @@ def get_cross_spectrum_mem(
     """
     # Calculate the autocorrelation of the time series
     autocorr = np.correlate(signal_0, signal_1, mode="full") / len(signal_0)
-    autocorr = autocorr[len(autocorr) //
-                        2: len(autocorr) // 2 + model_order + 1]
+    autocorr = autocorr[len(autocorr) // 2 : len(autocorr) // 2 + model_order + 1]
 
     # Create a Toeplitz matrix from the autocorrelation function
     # R = toeplitz(autocorr[:-1])
@@ -309,8 +309,7 @@ def get_line_widths(
     res = [np.nan, np.nan, np.nan]
 
     if use_lorentzian:
-        res = lorentzian_fit(frequencies, power_spectrum,
-                             filter_maximum=filter_maximum)
+        res = lorentzian_fit(frequencies, power_spectrum, filter_maximum=filter_maximum)
 
     if np.isnan(res[0]):
         res = get_peak_parameters(frequencies, power_spectrum)
@@ -396,8 +395,7 @@ def _get_normal_mode_decomposition_numba(
             # Loop over atoms and components
             for i in range(number_of_cell_atoms):
                 for m in range(velocity_components):
-                    projection_sum += velocities[n,
-                                                 i, m] * eigenvectors[k, i, m]
+                    projection_sum += velocities[n, i, m] * eigenvectors[k, i, m]
 
             # Store the result in the projected velocities array
             velocities_projected[n, k] = projection_sum
@@ -408,9 +406,7 @@ def _get_normal_mode_decomposition_numpy(
 ) -> None:
 
     # Use einsum to perform the double summation over cell atoms and time steps
-    velocities_projected += np.einsum(
-        'tij,kij->tk', velocities, eigenvectors.conj()
-    )
+    velocities_projected += np.einsum("tij,kij->tk", velocities, eigenvectors.conj())
 
     # number_of_cell_atoms = velocities.shape[1]
     # number_of_frequencies = eigenvectors.shape[0]

dfttoolkit/vibrations.py

@@ -74,9 +74,9 @@ def get_displacements(self, displacement: float = 0.0025) -> list:
 
         Parameters
         ----------
-        displacement : float, optional
+        displacement : float, default=0.0025
             Displacement for finte difference calculation of vibrations in
-            Angstrom. The default is 0.0025 Angstrom.
+            Angstrom.
 
         Returns
         -------
@@ -133,9 +133,8 @@ def get_hessian(
 
         Parameters
         ----------
-        set_constrained_atoms_zero : bool, optional
+        set_constrained_atoms_zero : bool, default=False
             Set elements in Hessian that code for constrained atoms to zero.
-            The default is False.
 
         Returns
         -------
@@ -185,8 +184,8 @@ def get_symmetrized_hessian(self, hessian=None):
 
         Parameters
         ----------
-        hessian : TYPE, optional
-            DESCRIPTION. The default is None.
+        hessian : TYPE, default=None
+            DESCRIPTION
 
         Returns
         -------
@@ -227,13 +226,12 @@ def get_eigenvalues_and_eigenvectors(
         ----------
         hessian : npt.NDArray[np.float64], optional
             Hessian. The default is None.
-        only_real : bool, optional
+        only_real : bool, default=True
             Returns only real valued eigenfrequencies + eigenmodes
             (ATTENTION: if you want to also include instable modes, you have to
-            symmetrize the hessian as provided below). The default is True.
-        symmetrize_hessian : bool, optional
+            symmetrize the hessian as provided below).
+        symmetrize_hessian : bool, default=True
             Symmetrise the hessian only for this function (no global change).
-            The default is True.
 
         Returns
         -------
@@ -500,10 +498,10 @@ def get_cross_correlation_function(
             DESCRIPTION.
         time_step : float
             DESCRIPTION.
-        bootstrapping_blocks : int, optional
-            DESCRIPTION. The default is 1.
-        bootstrapping_overlap : int, optional
-            DESCRIOTION. The default is 0.
+        bootstrapping_blocks : int, default=1
+            DESCRIPTION
+        bootstrapping_overlap : int, default=0
+            DESCRIPTION
 
         Returns
         -------

tests/conftest.py

@@ -5,7 +5,7 @@ def pytest_addoption(parser):
     """Add custom command line options to the pytest command."""
 
     parser.addoption(
-        "--run_aims",
+        "--run-aims",
         nargs="?",
         const=True,
         default=False,
@@ -20,4 +20,4 @@ def pytest_addoption(parser):
 
 @pytest.fixture(scope="session")
 def run_aims(request):
-    return request.config.getoption("--run_aims")
+    return request.config.getoption("--run-aims")

tests/test_output.py

@@ -15,9 +15,9 @@ def aims_out(self, request, run_aims):
 
         cwd = os.path.dirname(os.path.realpath(__file__))
 
-        # Run the FHI-aims calculations if the run_aims option is specified but not if
+        # Run the FHI-aims calculations if the run-aims option is specified but not if
         # they already exist.
-        # Force it to run if the run_aims option is "change_bin"
+        # Force it to run if the run-aims option is "change_bin"
         # run_aims fixture is defined in conftest.py
         if request.param == 1 and run_aims is not False:
             binary = aims_bin_path_prompt(run_aims, cwd)
@@ -50,10 +50,10 @@ def test_get_geometry(self):
 
         if self._aims_fixture_no in [1, 2, 3, 5, 7, 9]:
             assert len(geom) == 3
-            assert geom.get_is_periodic() == False
+            assert geom.get_is_periodic() is False
         else:
             assert len(geom) == 2
-            assert geom.get_is_periodic() == True
+            assert geom.get_is_periodic() is True
 
     # TODO
     # def test_get_parameters(self):

tests/test_utils.py

@@ -0,0 +1,52 @@
+import pytest
+import os
+from dfttoolkit.utils.run_utils import no_repeat
+
+
+class TestNoRepeat:
+
+    calc_dir = (
+        f"{os.path.dirname(os.path.realpath(__file__))}/fixtures/default_aims_calcs/1"
+    )
+
+    def test_specified_dir_not_found(self):
+        @no_repeat(calc_dir="bogus")
+        def to_be_decorated():
+            return True
+
+        with pytest.raises(ValueError) as excinfo:
+            to_be_decorated()
+
+        assert "bogus is not a directory." == str(excinfo.value)
+
+    @pytest.mark.parametrize(
+        ("calc_dir", "force", "expected"),
+        [
+            ("./", False, True),
+            (
+                f"{os.path.dirname(os.path.realpath(__file__))}/fixtures/default_aims_calcs/1",
+                True,
+                True,
+            ),
+        ],
+    )
+    def test_default_dir_no_args(self, calc_dir, force, expected):
+        @no_repeat(calc_dir=calc_dir, force=force)
+        def to_be_decorated():
+            return True
+
+        assert to_be_decorated() == expected
+
+    def test_no_repeat(self, capfd):
+        @no_repeat(calc_dir=self.calc_dir)
+        def to_be_decorated():
+            return True
+
+        to_be_decorated()
+
+        out, err = capfd.readouterr()
+        assert (
+            out
+            == f"aims.out already exists in {self.calc_dir}. Skipping calculation.\n"
+        )
+        assert err == ""