From de31609a42bcbfacf84fdc8d7da8313563e5a685 Mon Sep 17 00:00:00 2001 From: Edward Linscott Date: Tue, 7 May 2024 10:05:34 +0200 Subject: [PATCH] Autotidying --- bin/update_cff.py | 2 ++ src/koopmans/calculators/_koopmans_ham.py | 2 +- src/koopmans/calculators/_ph.py | 2 +- src/koopmans/calculators/_projwfc.py | 2 +- src/koopmans/calculators/_pw.py | 4 +-- src/koopmans/calculators/_utils.py | 2 +- src/koopmans/cli/main.py | 3 +-- src/koopmans/settings/_ph.py | 2 +- src/koopmans/settings/_wannier90.py | 7 ++--- src/koopmans/utils/_xml.py | 14 +++++----- src/koopmans/utils/_xsf.py | 2 +- src/koopmans/workflows/_convergence_ml.py | 4 +-- src/koopmans/workflows/_koopmans_dfpt.py | 4 +-- src/koopmans/workflows/_koopmans_dscf.py | 4 +-- src/koopmans/workflows/_singlepoint.py | 2 +- src/koopmans/workflows/_trajectory.py | 2 +- src/koopmans/workflows/_wannierize.py | 2 +- src/koopmans/workflows/_workflow.py | 12 ++++----- tests/calculators/test_ph.py | 2 -- tests/calculators/test_projwfc.py | 2 -- tests/conftest.py | 6 +++-- tests/helpers/os.py | 1 + tests/helpers/patches/__init__.py | 24 ++++++++---------- tests/helpers/patches/_check.py | 4 +-- tests/io/test_io_json.py | 1 + tests/test_cell.py | 1 - tests/test_kpoints.py | 3 +-- tests/test_pseudopotentials.py | 1 + tests/workflows/test_ui.py | 2 -- .../2x2x2/si_wannierize_bandstructure.png | Bin 1144363 -> 1144363 bytes tutorials/tutorial_4/convergence.png | Bin 46494 -> 46494 bytes tutorials/tutorial_4/custom_convergence.py | 6 +++++ tutorials/tutorial_4/plot.py | 3 +++ tutorials/tutorial_5/perturb_positions.py | 1 - 34 files changed, 66 insertions(+), 63 deletions(-) diff --git a/bin/update_cff.py b/bin/update_cff.py index bd03229a1..fce504bd4 100644 --- a/bin/update_cff.py +++ b/bin/update_cff.py @@ -1,5 +1,7 @@ import sys + import yaml + if sys.version_info >= (3, 8): from importlib import metadata else: diff --git a/src/koopmans/calculators/_koopmans_ham.py b/src/koopmans/calculators/_koopmans_ham.py index babfe2736..e1b0827a3 100644 --- a/src/koopmans/calculators/_koopmans_ham.py +++ b/src/koopmans/calculators/_koopmans_ham.py @@ -52,7 +52,7 @@ def write_alphas(self): def _pre_calculate(self): super()._pre_calculate() self.write_alphas() - + def _post_calculate(self): super()._post_calculate() if isinstance(self.parameters.kpts, BandPath) and len(self.parameters.kpts.kpts) > 1: diff --git a/src/koopmans/calculators/_ph.py b/src/koopmans/calculators/_ph.py index 7b0ff0799..f6a1a77a4 100644 --- a/src/koopmans/calculators/_ph.py +++ b/src/koopmans/calculators/_ph.py @@ -14,7 +14,7 @@ from ase import Atoms from ase.calculators.espresso import EspressoPh -from koopmans.commands import ParallelCommand, Command +from koopmans.commands import Command, ParallelCommand from koopmans.settings import PhSettingsDict from ._utils import CalculatorABC, CalculatorExt diff --git a/src/koopmans/calculators/_projwfc.py b/src/koopmans/calculators/_projwfc.py index 2ffb75232..6ec13f6e6 100644 --- a/src/koopmans/calculators/_projwfc.py +++ b/src/koopmans/calculators/_projwfc.py @@ -55,7 +55,7 @@ def _pre_calculate(self): if not hasattr(self, attr): raise ValueError(f'Please set {self.__class__.__name__}.{attr} before calling ' f'{self.__class__.__name__.calculate()}') - + def _post_calculate(self): super()._post_calculate() self.generate_dos() diff --git a/src/koopmans/calculators/_pw.py b/src/koopmans/calculators/_pw.py index c5ce88102..9a3f539f5 100644 --- a/src/koopmans/calculators/_pw.py +++ b/src/koopmans/calculators/_pw.py @@ -50,7 +50,7 @@ def _pre_calculate(self): if not isinstance(self.parameters.kpts, BandPath): raise KeyError('You are running a calculation that requires a kpoint path; please provide a BandPath ' 'as the kpts parameter') - + super()._pre_calculate() return @@ -62,7 +62,7 @@ def _post_calculate(self): if isinstance(self.parameters.kpts, BandPath): # Add the bandstructure to the results. This is very un-ASE-y and might eventually be replaced self.generate_band_structure() - + return def is_complete(self): diff --git a/src/koopmans/calculators/_utils.py b/src/koopmans/calculators/_utils.py index 39ef7562e..1072e6510 100644 --- a/src/koopmans/calculators/_utils.py +++ b/src/koopmans/calculators/_utils.py @@ -162,7 +162,7 @@ def _pre_calculate(self): def _calculate(self): """Run the calculation using the ASE calculator's calculate() method - + This method should NOT be overwritten by child classes. Child classes should only modify _pre_calculate() and _post_calculate() to perform any necessary pre- and post-calculation steps.""" diff --git a/src/koopmans/cli/main.py b/src/koopmans/cli/main.py index 24b98083c..6df4ea210 100755 --- a/src/koopmans/cli/main.py +++ b/src/koopmans/cli/main.py @@ -1,7 +1,7 @@ #!/usr/bin/env python3 -import sys import argparse +import sys import textwrap import koopmans.mpl_config @@ -28,7 +28,6 @@ def main(): # Set traceback behaviour if not args.traceback: sys.tracebacklimit = 0 - # Run workflow workflow.run() diff --git a/src/koopmans/settings/_ph.py b/src/koopmans/settings/_ph.py index f34f95d9a..c3a873bc0 100644 --- a/src/koopmans/settings/_ph.py +++ b/src/koopmans/settings/_ph.py @@ -5,7 +5,7 @@ class PhSettingsDict(SettingsDict): def __init__(self, **kwargs) -> None: super().__init__(valid=['outdir', 'prefix', 'epsil', 'amass', 'fildyn', 'tr2_ph', 'trans'], - defaults={'outdir': 'TMP', 'prefix': 'kc', 'epsil': True, 'trans' : False + defaults={'outdir': 'TMP', 'prefix': 'kc', 'epsil': True, 'trans': False }, are_paths=['outdir'], **kwargs) diff --git a/src/koopmans/settings/_wannier90.py b/src/koopmans/settings/_wannier90.py index 7dbfd799c..0556a4508 100644 --- a/src/koopmans/settings/_wannier90.py +++ b/src/koopmans/settings/_wannier90.py @@ -2,7 +2,8 @@ import numpy as np from ase.dft.kpoints import BandPath, monkhorst_pack -from ase.io.wannier90 import construct_kpoint_path, proj_string_to_dict, formatted_str_to_list +from ase.io.wannier90 import (construct_kpoint_path, formatted_str_to_list, + proj_string_to_dict) from ._utils import SettingsDict @@ -55,8 +56,8 @@ def __setitem__(self, key: str, value: Any): self.kpoints = kpts elif key == 'koffset': if self.mp_grid is None: - raise ValueError('Cannot offset the list of k-points if "kpoints" has not been defined yet. ' + \ - 'Check that "kgrid" is provided before "koffset"') + raise ValueError('Cannot offset the list of k-points if "kpoints" has not been defined yet. ' + + 'Check that "kgrid" is provided before "koffset"') if isinstance(value, int) or isinstance(value, list) and all([isinstance(k, int) for k in value]): # For koffset = [1, 1, 1], PW shifts the k-grid by half a grid step self.kpoints += np.array(value) / self.mp_grid / 2 diff --git a/src/koopmans/utils/_xml.py b/src/koopmans/utils/_xml.py index e13401636..b15664d74 100644 --- a/src/koopmans/utils/_xml.py +++ b/src/koopmans/utils/_xml.py @@ -36,18 +36,18 @@ def read_xml_nr(xml_file: Path, string: str = 'EFFECTIVE-POTENTIAL') -> List[int def read_xml_array( - xml_file: Path, norm_const: float, string: str = 'EFFECTIVE-POTENTIAL', retain_final_element: bool=False - ) -> np.ndarray: + xml_file: Path, norm_const: float, string: str = 'EFFECTIVE-POTENTIAL', retain_final_element: bool = False +) -> np.ndarray: """ Loads an array from an xml file. - + :param xml_file: The xml file to read from :param norm_const: The normalization constant to multiply the array with (in our case 1/((Bohr radii)^3) - :param string: The name of the field in the xml file that contains the array, in our case either + :param string: The name of the field in the xml file that contains the array, in our case either 'EFFECTIVE-POTENTIAL' or 'CHARGE-DENSITY' - :param retain_final_element: If True, the array is returned in with periodic boundary conditions, i.e. the last + :param retain_final_element: If True, the array is returned in with periodic boundary conditions, i.e. the last element in each dimension is equal to the first element in each dimension. This is required for the xsf format. - + :return: The array """ @@ -71,7 +71,7 @@ def read_xml_array( array_xml[k, j, i] = rho_tmp[(j % (nr_xml[1]-1))*(nr_xml[0]-1)+(i % (nr_xml[0]-1))] array_xml *= norm_const - if retain_final_element: + if retain_final_element: # the xsf format requires an array where the last element is equal to the first element in each dimension return array_xml else: diff --git a/src/koopmans/utils/_xsf.py b/src/koopmans/utils/_xsf.py index 2024e2bc4..84604a670 100644 --- a/src/koopmans/utils/_xsf.py +++ b/src/koopmans/utils/_xsf.py @@ -28,7 +28,7 @@ def write_xsf(filename: Path, atoms: Atoms, arrays: List[np.ndarray], nr_xml: Tu out.write('CRYSTAL\n\n') out.write('PRIMVEC\n\n') for vec in cell_parameters: - out.write("\t" + " ".join([f"{x:13.10f}" for x in vec])+ " \n") + out.write("\t" + " ".join([f"{x:13.10f}" for x in vec]) + " \n") out.write('PRIMCOORD\n') out.write(f"\t{len(symbols)}\t1\n") for symbol, pos in zip(symbols, positions): diff --git a/src/koopmans/workflows/_convergence_ml.py b/src/koopmans/workflows/_convergence_ml.py index 30128574c..3d64d7d8b 100644 --- a/src/koopmans/workflows/_convergence_ml.py +++ b/src/koopmans/workflows/_convergence_ml.py @@ -134,9 +134,9 @@ def convert_to_list(param, type): return [param] else: # If param is not an int or a float check that it is a list of ints / floats - assert(isinstance(param, list)) + assert (isinstance(param, list)) for value in param: - assert(isinstance(value, type)) + assert (isinstance(value, type)) return param n_maxs = convert_to_list(self.ml.n_max, int) diff --git a/src/koopmans/workflows/_koopmans_dfpt.py b/src/koopmans/workflows/_koopmans_dfpt.py index 9131208cf..eda421c47 100644 --- a/src/koopmans/workflows/_koopmans_dfpt.py +++ b/src/koopmans/workflows/_koopmans_dfpt.py @@ -10,7 +10,7 @@ from pathlib import Path from typing import Dict -from koopmans import utils, pseudopotentials +from koopmans import pseudopotentials, utils from koopmans.bands import Bands from koopmans.calculators import (KoopmansHamCalculator, PWCalculator, Wann2KCCalculator) @@ -144,7 +144,7 @@ def _run(self): if key.startswith('w90'): self.calculator_parameters[key].write_u_matrices = True self.calculator_parameters[key].write_xyz = True - wf_workflow = WannierizeWorkflow.fromparent(self, force_nspin2=True, scf_kgrid = self._scf_kgrid) + wf_workflow = WannierizeWorkflow.fromparent(self, force_nspin2=True, scf_kgrid=self._scf_kgrid) wf_workflow.run() else: diff --git a/src/koopmans/workflows/_koopmans_dscf.py b/src/koopmans/workflows/_koopmans_dscf.py index e0cced0c3..c48bafb37 100644 --- a/src/koopmans/workflows/_koopmans_dscf.py +++ b/src/koopmans/workflows/_koopmans_dscf.py @@ -642,13 +642,13 @@ def perform_alpha_calculations(self) -> None: alpha, error = self.calculate_alpha_from_list_of_calcs( calcs, trial_calc, band, filled=band.filled) - + # Mixing alpha = self.parameters.alpha_mixing * alpha + (1 - self.parameters.alpha_mixing) * band.alpha warning_message = 'The computed screening parameter is {0}. Proceed with caution.' failure_message = 'The computed screening parameter is significantly {0}. This should not ' \ - 'happen. Decrease alpha_mixing and/or change alpha_guess.' + 'happen. Decrease alpha_mixing and/or change alpha_guess.' if alpha < -0.1: raise ValueError(failure_message.format('less than 0')) diff --git a/src/koopmans/workflows/_singlepoint.py b/src/koopmans/workflows/_singlepoint.py index 0db43748b..2d8d63f79 100644 --- a/src/koopmans/workflows/_singlepoint.py +++ b/src/koopmans/workflows/_singlepoint.py @@ -14,9 +14,9 @@ import numpy as np from koopmans import utils +from koopmans.calculators import ProjwfcCalculator from ._workflow import Workflow -from koopmans.calculators import ProjwfcCalculator load_results_from_output = True diff --git a/src/koopmans/workflows/_trajectory.py b/src/koopmans/workflows/_trajectory.py index a3c56e831..fa1703fc8 100644 --- a/src/koopmans/workflows/_trajectory.py +++ b/src/koopmans/workflows/_trajectory.py @@ -97,7 +97,7 @@ def _run(self): snapshot = self.snapshots[i] self.ml.current_snapshot = i self.atoms.set_positions(snapshot.positions) - + # after each snapshot we want to set the from_scratch_parameter to its original value # To do so, we save it here since it might get set from False to True during the calculation # of the snapshot diff --git a/src/koopmans/workflows/_wannierize.py b/src/koopmans/workflows/_wannierize.py index d04ec1153..494d050dd 100644 --- a/src/koopmans/workflows/_wannierize.py +++ b/src/koopmans/workflows/_wannierize.py @@ -11,7 +11,7 @@ import math import shutil from pathlib import Path -from typing import List, TypeVar, Optional +from typing import List, Optional, TypeVar # isort: off import koopmans.mpl_config diff --git a/src/koopmans/workflows/_workflow.py b/src/koopmans/workflows/_workflow.py index 97a26ba04..67f213533 100644 --- a/src/koopmans/workflows/_workflow.py +++ b/src/koopmans/workflows/_workflow.py @@ -568,9 +568,9 @@ def convert_to_list(param, type): if isinstance(param, type): # if param is an int or a float convert it for the checks to a list return [param] else: # if param is not an int or a float check that it is a list of ints / floats - assert(isinstance(param, list)) + assert (isinstance(param, list)) for value in param: - assert(isinstance(value, type)) + assert (isinstance(value, type)) return param n_maxs = convert_to_list(self.ml.n_max, int) @@ -718,7 +718,7 @@ def supercell_to_primitive(self, matrix: Optional[npt.NDArray[np.int_]] = None): def run_calculator(self, master_calc: calculators.Calc, enforce_spin_symmetry: bool = False): ''' Run a calculator. - + If enforce_spin_symmetry is True, the calculation will be run with spin symmetry enforced. Ultimately this wraps self.run_calculators @@ -800,7 +800,7 @@ def _pre_run_calculator(self, qe_calc: calculators.Calc) -> bool: qe_calc.command.postfix = f'-npool {self.parameters.npool}' return True - + def _run_calculators(self, calcs: List[calculators.Calc]) -> None: """Run a list of calculators, without doing anything else (other than printing messages.) @@ -842,7 +842,7 @@ def _post_run_calculator(self, calc: calculators.Calc) -> None: self.parameters.from_scratch = True return - + def run_calculators(self, calcs: List[calculators.Calc]): ''' Run a list of *independent* calculators (default implementation is to run them in sequence) @@ -855,7 +855,7 @@ def run_calculators(self, calcs: List[calculators.Calc]): calcs_to_run.append(calc) self._run_calculators(calcs_to_run) - + for calc in calcs_to_run: self._post_run_calculator(calc) diff --git a/tests/calculators/test_ph.py b/tests/calculators/test_ph.py index 30677a421..f4f91e1c8 100644 --- a/tests/calculators/test_ph.py +++ b/tests/calculators/test_ph.py @@ -6,8 +6,6 @@ from koopmans import utils, workflows from koopmans.io import read_kwf as read_encoded_json from koopmans.io import write_kwf as write_encoded_json - - from tests.helpers.patches import benchmark_filename diff --git a/tests/calculators/test_projwfc.py b/tests/calculators/test_projwfc.py index 6b29e6b7d..17128dce2 100644 --- a/tests/calculators/test_projwfc.py +++ b/tests/calculators/test_projwfc.py @@ -7,8 +7,6 @@ from koopmans import utils, workflows from koopmans.io import read_kwf as read_encoded_json from koopmans.io import write_kwf as write_encoded_json - - from tests.helpers.patches import benchmark_filename diff --git a/tests/conftest.py b/tests/conftest.py index d7fd1cf5c..aead26620 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,10 +1,12 @@ from pathlib import Path + import pytest -from .helpers.systems import gaas, ozone, silicon, tio2, water from .helpers.os import datadir, sys2file -from .helpers.patches import tutorial_patch, workflow_patch, ui_patch, espresso_patch +from .helpers.patches import (espresso_patch, tutorial_patch, ui_patch, + workflow_patch) from .helpers.strategies import ase_cells, bandpaths, kpoints +from .helpers.systems import gaas, ozone, silicon, tio2, water def pytest_addoption(parser): diff --git a/tests/helpers/os.py b/tests/helpers/os.py index 237355841..cb3a02839 100644 --- a/tests/helpers/os.py +++ b/tests/helpers/os.py @@ -1,4 +1,5 @@ from pathlib import Path + import pytest diff --git a/tests/helpers/patches/__init__.py b/tests/helpers/patches/__init__.py index 24099d914..75b54e452 100644 --- a/tests/helpers/patches/__init__.py +++ b/tests/helpers/patches/__init__.py @@ -1,25 +1,22 @@ import pytest -from ._check import (CheckEnvironCalculator, CheckKoopmansCPCalculator, - CheckKoopmansHamCalculator, CheckKoopmansScreenCalculator, - CheckMLFittingWorkflow, CheckPhCalculator, - CheckProjwfcCalculator, CheckPW2WannierCalculator, - CheckPWCalculator, CheckUnfoldAndInterpolateCalculator, - CheckWann2KCCalculator, CheckWann2KCPCalculator, - CheckWannier90Calculator) from ._benchmark import (BenchGenEnvironCalculator, BenchGenKoopmansCPCalculator, BenchGenKoopmansHamCalculator, BenchGenKoopmansScreenCalculator, - BenchGenMLFittingWorkflow, - BenchGenPhCalculator, + BenchGenMLFittingWorkflow, BenchGenPhCalculator, BenchGenProjwfcCalculator, - BenchGenPW2WannierCalculator, - BenchGenPWCalculator, + BenchGenPW2WannierCalculator, BenchGenPWCalculator, BenchGenUnfoldAndInterpolateCalculator, - BenchGenWann2KCCalculator, - BenchGenWann2KCPCalculator, + BenchGenWann2KCCalculator, BenchGenWann2KCPCalculator, BenchGenWannier90Calculator) +from ._check import (CheckEnvironCalculator, CheckKoopmansCPCalculator, + CheckKoopmansHamCalculator, CheckKoopmansScreenCalculator, + CheckMLFittingWorkflow, CheckPhCalculator, + CheckProjwfcCalculator, CheckPW2WannierCalculator, + CheckPWCalculator, CheckUnfoldAndInterpolateCalculator, + CheckWann2KCCalculator, CheckWann2KCPCalculator, + CheckWannier90Calculator) from ._mock import (MockEnvironCalculator, MockKoopmansCPCalculator, MockKoopmansDSCFWorkflow, MockKoopmansHamCalculator, MockKoopmansScreenCalculator, MockMLFittingWorkflow, @@ -39,7 +36,6 @@ StumblingTrajectoryWorkflow, StumblingUnfoldAndInterpolateWorkflow, StumblingWannierizeWorkflow) - from ._utils import benchmark_filename diff --git a/tests/helpers/patches/_check.py b/tests/helpers/patches/_check.py index 5181d83da..20317b731 100644 --- a/tests/helpers/patches/_check.py +++ b/tests/helpers/patches/_check.py @@ -166,7 +166,7 @@ def _print_messages(self, messages: List[Dict[str, str]]) -> None: message = message.replace('disagreements', 'disagreement') raise CalculationFailed(message) - + def _load_benchmark(self) -> Calc: with utils.chdir(self.directory): # type: ignore[attr-defined] # By moving into the directory where the calculation was run, we ensure when we read in the settings that @@ -177,7 +177,7 @@ def _load_benchmark(self) -> Calc: def _pre_calculate(self): """Before running the calculation, check the settings are the same""" - + # Perform the pre_calculate first, as sometimes this function modifies the input parameters super()._pre_calculate() diff --git a/tests/io/test_io_json.py b/tests/io/test_io_json.py index f09bb698f..02173ac44 100644 --- a/tests/io/test_io_json.py +++ b/tests/io/test_io_json.py @@ -5,6 +5,7 @@ ''' from ase.build import molecule + from koopmans import utils from koopmans.io import read, write from koopmans.workflows import (ConvergenceMLWorkflow, SinglepointWorkflow, diff --git a/tests/test_cell.py b/tests/test_cell.py index 5269f46ec..f8aea2023 100644 --- a/tests/test_cell.py +++ b/tests/test_cell.py @@ -6,7 +6,6 @@ from hypothesis import given, settings, strategies from koopmans.cell import cell_to_parameters, parameters_to_cell - from tests.helpers import strategies as kst # Reference list of ASE Bravais lattice classes diff --git a/tests/test_kpoints.py b/tests/test_kpoints.py index 1c3b48a96..696fc7380 100644 --- a/tests/test_kpoints.py +++ b/tests/test_kpoints.py @@ -1,10 +1,9 @@ import numpy as np +import pytest from ase.dft.kpoints import BandPath from hypothesis import given, settings -import pytest from koopmans.kpoints import Kpoints, dict_to_kpath, kpath_to_dict - from tests.helpers import strategies as kst diff --git a/tests/test_pseudopotentials.py b/tests/test_pseudopotentials.py index fd7c4d805..f61dc7191 100644 --- a/tests/test_pseudopotentials.py +++ b/tests/test_pseudopotentials.py @@ -1,4 +1,5 @@ from ase import Atoms + from koopmans import pseudopotentials diff --git a/tests/workflows/test_ui.py b/tests/workflows/test_ui.py index cb9bd59ed..440b2f0f0 100644 --- a/tests/workflows/test_ui.py +++ b/tests/workflows/test_ui.py @@ -3,8 +3,6 @@ from koopmans import workflows from koopmans.io import read_kwf as read_encoded_json from koopmans.utils import chdir - - from tests.helpers.patches import benchmark_filename diff --git a/tutorials/tutorial_2/2x2x2/si_wannierize_bandstructure.png b/tutorials/tutorial_2/2x2x2/si_wannierize_bandstructure.png index d25afca4f1134314146577f77825d8cf05e56014..30d106daaa347f33f2afc8b9a06c3118decfe997 100644 GIT binary patch delta 96 zcmZ4e#&z`@*9mS47J7y{3K=CO1;tkS`nicE1v&X8Ihjd%`9#J Ai~s-t delta 96 zcmZ4e#&z`@*9mS4rh0}t3K=CO1;tkS`nicE1v&X8Ihjd%`9