Skip to content

Commit

Permalink
Autotidying
Browse files Browse the repository at this point in the history
  • Loading branch information
elinscott committed May 7, 2024
1 parent 3f1432d commit de31609
Show file tree
Hide file tree
Showing 34 changed files with 66 additions and 63 deletions.
2 changes: 2 additions & 0 deletions bin/update_cff.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import sys

import yaml

if sys.version_info >= (3, 8):
from importlib import metadata
else:
Expand Down
2 changes: 1 addition & 1 deletion src/koopmans/calculators/_koopmans_ham.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion src/koopmans/calculators/_ph.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/koopmans/calculators/_projwfc.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions src/koopmans/calculators/_pw.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion src/koopmans/calculators/_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""

Expand Down
3 changes: 1 addition & 2 deletions src/koopmans/cli/main.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/usr/bin/env python3

import sys
import argparse
import sys
import textwrap

import koopmans.mpl_config
Expand All @@ -28,7 +28,6 @@ def main():
# Set traceback behaviour
if not args.traceback:
sys.tracebacklimit = 0


# Run workflow
workflow.run()
2 changes: 1 addition & 1 deletion src/koopmans/settings/_ph.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 4 additions & 3 deletions src/koopmans/settings/_wannier90.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
14 changes: 7 additions & 7 deletions src/koopmans/utils/_xml.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
"""

Expand All @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion src/koopmans/utils/_xsf.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
4 changes: 2 additions & 2 deletions src/koopmans/workflows/_convergence_ml.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions src/koopmans/workflows/_koopmans_dfpt.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions src/koopmans/workflows/_koopmans_dscf.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'))
Expand Down
2 changes: 1 addition & 1 deletion src/koopmans/workflows/_singlepoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion src/koopmans/workflows/_trajectory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/koopmans/workflows/_wannierize.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions src/koopmans/workflows/_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.)
Expand Down Expand Up @@ -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)
Expand All @@ -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)

Expand Down
2 changes: 0 additions & 2 deletions tests/calculators/test_ph.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
2 changes: 0 additions & 2 deletions tests/calculators/test_projwfc.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
6 changes: 4 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -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):
Expand Down
1 change: 1 addition & 0 deletions tests/helpers/os.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from pathlib import Path

import pytest


Expand Down
24 changes: 10 additions & 14 deletions tests/helpers/patches/__init__.py
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -39,7 +36,6 @@
StumblingTrajectoryWorkflow,
StumblingUnfoldAndInterpolateWorkflow,
StumblingWannierizeWorkflow)

from ._utils import benchmark_filename


Expand Down
4 changes: 2 additions & 2 deletions tests/helpers/patches/_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()

Expand Down
1 change: 1 addition & 0 deletions tests/io/test_io_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion tests/test_cell.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit de31609

Please sign in to comment.