diff --git a/.github/workflows/sphinx-docs.yaml b/.github/workflows/sphinx-docs.yaml index 2085d93..7193ab1 100644 --- a/.github/workflows/sphinx-docs.yaml +++ b/.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 }} diff --git a/README.md b/README.md index c7f6d80..da4730a 100644 --- a/README.md +++ b/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 diff --git a/dfttoolkit/output.py b/dfttoolkit/output.py index 84797a1..1b49606 100644 --- a/dfttoolkit/output.py +++ b/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 ------- diff --git a/dfttoolkit/utils/math_utils.py b/dfttoolkit/utils/math_utils.py index 23cad79..c8af90b 100644 --- a/dfttoolkit/utils/math_utils.py +++ b/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 ------- diff --git a/dfttoolkit/utils/run_utils.py b/dfttoolkit/utils/run_utils.py index 056198b..9400aca 100644 --- a/dfttoolkit/utils/run_utils.py +++ b/dfttoolkit/utils/run_utils.py @@ -1,14 +1,26 @@ 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 ------- @@ -16,25 +28,21 @@ def no_repeat(func): 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 diff --git a/dfttoolkit/utils/vibrations_utils.py b/dfttoolkit/utils/vibrations_utils.py index d390138..4df3f58 100644 --- a/dfttoolkit/utils/vibrations_utils.py +++ b/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] diff --git a/dfttoolkit/vibrations.py b/dfttoolkit/vibrations.py index 0e97387..2fb9b77 100644 --- a/dfttoolkit/vibrations.py +++ b/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 ------- diff --git a/tests/conftest.py b/tests/conftest.py index 40d079c..8e85d8a 100644 --- a/tests/conftest.py +++ b/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") diff --git a/tests/test_output.py b/tests/test_output.py index 3f20df5..0f5c9f6 100644 --- a/tests/test_output.py +++ b/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): diff --git a/tests/test_utils.py b/tests/test_utils.py new file mode 100644 index 0000000..8d57230 --- /dev/null +++ b/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 == ""