From f711ec2d90d593bf4554bc049c29ecadb38d58cc Mon Sep 17 00:00:00 2001 From: Edward Linscott Date: Thu, 29 Sep 2022 15:51:35 +0200 Subject: [PATCH 1/4] Fixing license info in pyproject.toml --- pyproject.toml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 93ea11c21..afbe9416e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,13 +4,12 @@ version = "1.0.0-beta.3" description = "Koopmans spectral functional calculations with python and Quantum ESPRESSO" readme = "README.rst" requires-python = ">=3.7" -license = { name = "GPL", file = "LICENSE" } authors = [{ name = "Edward Linscott", email = "edward.linscott@epfl.ch" }, { name = "Riccardo De Gennaro", email = "riccardo.degennaro@epfl.ch" }, { name = "Nicola Colonna", email = "nicola.colonna@psi.ch" }] classifiers = [ "Intended Audience :: Science/Research", - "License :: OSI Approved :: MIT License", + "License :: OSI Approved :: GNU General Public License v2 (GPLv2)", "Programming Language :: Python :: 3", "Programming Language :: Python :: 3.7", "Programming Language :: Python :: 3.8", From 4ea92dd4d51eda7d85e0e78b91f1c050bfc8f655 Mon Sep 17 00:00:00 2001 From: Edward Linscott Date: Thu, 29 Sep 2022 16:39:55 +0200 Subject: [PATCH 2/4] Remove the algebraic parsing of settings (it wasn't robust) and replace with a specific search-and-replace for nelec and nelec only --- src/koopmans/settings/_koopmans_cp.py | 1 - src/koopmans/settings/_koopmans_ham.py | 1 - src/koopmans/settings/_koopmans_screen.py | 1 - src/koopmans/settings/_pw.py | 1 - src/koopmans/settings/_pw2wannier.py | 1 - src/koopmans/settings/_ui.py | 4 +- src/koopmans/settings/_utils.py | 66 ++++------------------- src/koopmans/settings/_wann2kc.py | 1 - src/koopmans/settings/_wann2kcp.py | 1 - src/koopmans/settings/_wannier90.py | 1 - src/koopmans/workflows/_workflow.py | 7 ++- 11 files changed, 15 insertions(+), 70 deletions(-) diff --git a/src/koopmans/settings/_koopmans_cp.py b/src/koopmans/settings/_koopmans_cp.py index d52de88f0..53163aca7 100644 --- a/src/koopmans/settings/_koopmans_cp.py +++ b/src/koopmans/settings/_koopmans_cp.py @@ -47,7 +47,6 @@ def __init__(self, **kwargs) -> None: super().__init__(valid=[k for sublist in kcp_keys.values() for k in sublist], defaults=defaults, are_paths=['outdir', 'pseudo_dir'], - to_not_parse=['assume_isolated'], **flattened_kwargs) @property diff --git a/src/koopmans/settings/_koopmans_ham.py b/src/koopmans/settings/_koopmans_ham.py index ef16c3f78..2d6346a01 100644 --- a/src/koopmans/settings/_koopmans_ham.py +++ b/src/koopmans/settings/_koopmans_ham.py @@ -12,7 +12,6 @@ def __init__(self, **kwargs) -> None: defaults={'calculation': 'ham', 'do_bands': True, 'use_ws_distance': True, 'write_hr': True, 'l_alpha_corr': False, **kc_wann_defaults}, are_paths=['outdir', 'pseudo_dir'], - to_not_parse=['assume_isolated'], **kwargs) @property diff --git a/src/koopmans/settings/_koopmans_screen.py b/src/koopmans/settings/_koopmans_screen.py index c06fea13b..10e7d7474 100644 --- a/src/koopmans/settings/_koopmans_screen.py +++ b/src/koopmans/settings/_koopmans_screen.py @@ -11,7 +11,6 @@ def __init__(self, **kwargs) -> None: defaults={'calculation': 'screen', 'tr2': 1.0e-18, 'nmix': 4, 'niter': 33, 'check_spread': True, **kc_wann_defaults}, are_paths=['outdir', 'pseudo_dir'], - to_not_parse=['assume_isolated'], ** kwargs) @property diff --git a/src/koopmans/settings/_pw.py b/src/koopmans/settings/_pw.py index 1007a9311..3504df12d 100644 --- a/src/koopmans/settings/_pw.py +++ b/src/koopmans/settings/_pw.py @@ -20,7 +20,6 @@ def __init__(self, **kwargs) -> None: defaults={'calculation': 'scf', 'outdir': './TMP/', 'prefix': 'kc', 'conv_thr': '2.0e-9*nelec', 'verbosity': 'high'}, are_paths=['outdir', 'pseudo_dir'], - to_not_parse=['assume_isolated'], **flattened_kwargs) def is_valid(self, name: str) -> bool: diff --git a/src/koopmans/settings/_pw2wannier.py b/src/koopmans/settings/_pw2wannier.py index 9c4d7ca87..a4b45450a 100644 --- a/src/koopmans/settings/_pw2wannier.py +++ b/src/koopmans/settings/_pw2wannier.py @@ -9,7 +9,6 @@ def __init__(self, **kwargs) -> None: defaults={'outdir': 'TMP', 'prefix': 'kc', 'seedname': 'wann', 'wan_mode': 'standalone'}, are_paths=['outdir'], - to_not_parse=['wan_mode'], **kwargs) @property diff --git a/src/koopmans/settings/_ui.py b/src/koopmans/settings/_ui.py index c23de2cd8..468cca4e4 100644 --- a/src/koopmans/settings/_ui.py +++ b/src/koopmans/settings/_ui.py @@ -61,9 +61,7 @@ class UnfoldAndInterpolateSettingsDict(SettingsDictWithChecks): def __init__(self, **kwargs): - super().__init__(settings=valid_settings, - to_not_parse=[], - **kwargs) + super().__init__(settings=valid_settings, **kwargs) @property def _other_valid_keywords(self): diff --git a/src/koopmans/settings/_utils.py b/src/koopmans/settings/_utils.py index 0118c5e54..04a3af4f0 100644 --- a/src/koopmans/settings/_utils.py +++ b/src/koopmans/settings/_utils.py @@ -16,7 +16,7 @@ from koopmans.utils import units -def parse_physical(value): +def parse_physical(value, nelec=None): ''' Takes in a value that potentially has a unit following a float, converts the value to ASE's default units (Ang, eV), and returns @@ -34,6 +34,9 @@ def parse_physical(value): elif len(splitline) == 2: [value, val_units] = splitline value = float(value) + if val_units == 'nelec': + # Leave 'nelec' untouched; it will be parsed later by replace_nelec() + return f'{value} nelec' matching_units = [ u for u in units if u.lower() == val_units.lower()] if len(matching_units) == 1: @@ -65,7 +68,6 @@ class SettingsDict(UserDict): valid -- list of valid settings defaults -- dict of defaults for each setting are_paths -- list of settings that correspond to paths - to_not_parse -- list of settings that should not be parsed algebraically directory -- the directory in which the calculation is being run (used to enforce all path settings to be absolute paths) physicals -- list of keywords that have accompanying units from input @@ -78,17 +80,15 @@ class SettingsDict(UserDict): valid: List[str] = [] data: Dict[str, Any] = {} are_paths: List[str] = [] - to_not_parse: List[str] = [] physicals: List[str] = [] def __init__(self, valid: List[str], defaults: Dict[str, Any] = {}, are_paths: List[str] = [], - to_not_parse: List[str] = [], directory='', physicals: List[str] = [], **kwargs): + directory='', physicals: List[str] = [], **kwargs): super().__init__() self.valid = valid + self._other_valid_keywords self.are_paths = are_paths self.defaults = {k: v for k, v in defaults.items() if k not in self.are_paths} self.defaults.update(**{k: Path(v) for k, v in defaults.items() if k in self.are_paths}) - self.to_not_parse = to_not_parse self.directory = directory if directory is not None else Path.cwd() self.physicals = physicals self.update(**defaults) @@ -174,56 +174,12 @@ def _check_before_setitem(self, key, value): raise KeyError(f'{key} is not a valid setting') return - def parse_algebraic_setting(self, expr, **kwargs): - # Checks if expr is defined algebraically, and evaluates them - if not isinstance(expr, str): - return expr - if all([c.isalpha() or c in ['_', '"', "'"] for c in expr]): - return expr.strip('"').strip("'") - - for replace in [('/', ' / '), ('*', ' * '), ('-', ' - '), ('+', ' + '), ('e - ', 'e-'), ('e + ', 'e+')]: - expr = expr.replace(*replace) - expr = expr.split() - - for i, term in enumerate(expr): - if term in ['*', '/', '+', '-']: - continue - elif all([c.isalpha() for c in term]): - if term in kwargs: - expr[i] = kwargs[term] - elif getattr(self, term, None) is None: - raise ValueError('Failed to parse ' + ''.join(map(str, expr))) - else: - expr[i] = getattr(self, term) - elif len(expr) == 1 and any([c.isalpha() for c in term]): - return term.strip('"').strip("'") - else: - expr[i] = float(term) - - value = float(expr[0]) - for op, term in zip(expr[1::2], expr[2::2]): - if op == '*': - value *= float(term) - elif op == '/': - value /= float(term) - elif op == '+': - value += float(term) - elif op == '-': - value -= float(term) - else: - raise ValueError('Failed to parse ' + ''.join([str(e) for e in expr])) - - return value - - def parse_algebraic_settings(self, **kwargs): - # Checks self.parameters for keywords defined algebraically, and evaluates them. - # kwargs can be used to provide values for keywords such as 'nelec', which the - # SettingsDict may not have access to but is a useful keyword for scaling extensive - # parameters - for key in self.data.keys(): - if key in self.to_not_parse: - continue - self.data[key] = self.parse_algebraic_setting(self.data[key], **kwargs) + def replace_nelec(self, nelec: int): + for k, v in self.items(): + if isinstance(v, str): + v_list = v.replace('*', ' ').split() + if len(v_list) == 2 and v_list[1] == 'nelec': + self[k] = float(v_list[0]) * nelec @property def _other_valid_keywords(self): diff --git a/src/koopmans/settings/_wann2kc.py b/src/koopmans/settings/_wann2kc.py index 787336e9b..971f79174 100644 --- a/src/koopmans/settings/_wann2kc.py +++ b/src/koopmans/settings/_wann2kc.py @@ -19,7 +19,6 @@ def __init__(self, **kwargs) -> None: super().__init__(valid=[k for block in w2kcw_keys.values() for k in block], defaults={'calculation': 'wann2kcw', **kc_wann_defaults}, are_paths=['outdir', 'pseudo_dir'], - to_not_parse=['assume_isolated'], **flattened_kwargs) @property diff --git a/src/koopmans/settings/_wann2kcp.py b/src/koopmans/settings/_wann2kcp.py index 1c2aa4b01..8cdc53bd5 100644 --- a/src/koopmans/settings/_wann2kcp.py +++ b/src/koopmans/settings/_wann2kcp.py @@ -8,7 +8,6 @@ def __init__(self, **kwargs) -> None: defaults={'outdir': 'TMP', 'prefix': 'kc', 'seedname': 'wann', 'wan_mode': 'wannier2kcp'}, are_paths=['outdir'], - to_not_parse=['wan_mode', 'wannier_plot_list'], **kwargs) @property diff --git a/src/koopmans/settings/_wannier90.py b/src/koopmans/settings/_wannier90.py index b405f3c03..b5b0ecc7b 100644 --- a/src/koopmans/settings/_wannier90.py +++ b/src/koopmans/settings/_wannier90.py @@ -19,7 +19,6 @@ def __init__(self, **kwargs) -> None: 'translation_centre_frac'], defaults={'num_iter': 10000, 'conv_tol': 1.e-10, 'conv_window': 5, 'write_hr': True, 'guiding_centres': True, 'gamma_only': False}, - to_not_parse=['exclude_bands', 'wannier_plot_list'], **kwargs) def update(self, *args, **kwargs) -> None: diff --git a/src/koopmans/workflows/_workflow.py b/src/koopmans/workflows/_workflow.py index f10118929..f1c4f29d1 100644 --- a/src/koopmans/workflows/_workflow.py +++ b/src/koopmans/workflows/_workflow.py @@ -282,10 +282,9 @@ def __init__(self, atoms: Atoms, 'and to instead double-check the keyword in the various .win files ' 'generated by the workflow.') - # Automatically parsing algebraic settings. We need to provide nelec explicitly for calculators such as - # PWCalculators, which don't have nelec as a setting but it is still useful for specifying settings - # algebraically - params.parse_algebraic_settings(nelec=nelec) + # Replace "nelec" as a unit with its numerical value, for any settings that are specified implicitly in + # terms of nelec + params.replace_nelec(nelec) # Store the sanitized parameters self.calculator_parameters[block] = params From 53d4e168275b6bcf32e2e126d4b35a07f15aec9a Mon Sep 17 00:00:00 2001 From: Edward Linscott Date: Fri, 30 Sep 2022 18:38:26 +0200 Subject: [PATCH 3/4] Remove erroneously included nelec arg to parse_physical --- src/koopmans/settings/_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/koopmans/settings/_utils.py b/src/koopmans/settings/_utils.py index 04a3af4f0..bd6b440a8 100644 --- a/src/koopmans/settings/_utils.py +++ b/src/koopmans/settings/_utils.py @@ -16,7 +16,7 @@ from koopmans.utils import units -def parse_physical(value, nelec=None): +def parse_physical(value): ''' Takes in a value that potentially has a unit following a float, converts the value to ASE's default units (Ang, eV), and returns From c50a70bfc332336fce1934c787cf024c071f2e07 Mon Sep 17 00:00:00 2001 From: degennar Date: Mon, 14 Nov 2022 15:03:12 +0100 Subject: [PATCH 4/4] fixing issue when defining the pseudopotentials block in the JSON --- src/koopmans/settings/_workflow.py | 2 +- src/koopmans/workflows/_workflow.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/koopmans/settings/_workflow.py b/src/koopmans/settings/_workflow.py index 936bb27d8..7225f1cc3 100644 --- a/src/koopmans/settings/_workflow.py +++ b/src/koopmans/settings/_workflow.py @@ -29,7 +29,7 @@ def __init__(self, **kwargs) -> None: str, None, None), Setting('pseudo_directory', 'the folder containing the pseudopotentials to use (mutually exclusive with "pseudo_library")', - (str, Path), None, None), + Path, None, None), Setting('method', 'the method to calculate the screening parameters: either with ΔSCF or DFPT', str, 'dscf', ('dscf', 'dfpt')), diff --git a/src/koopmans/workflows/_workflow.py b/src/koopmans/workflows/_workflow.py index e3177fb7d..b5c106bf9 100644 --- a/src/koopmans/workflows/_workflow.py +++ b/src/koopmans/workflows/_workflow.py @@ -69,7 +69,7 @@ class Workflow(ABC): atoms : Atoms an ASE ``Atoms`` object defining the atomic positions, cell, etc - pseudopotentals : Dict[str, str] + pseudopotentials : Dict[str, str] a dictionary mapping atom labels to pseudopotential filenames kpoints : koopmans.kpoints.Kpoints @@ -984,6 +984,8 @@ def fromjson(cls, fname: str, override: Dict[str, Any] = {}): # Define the name of the workflow using the name of the json file kwargs['name'] = fname.replace('.json', '') + kwargs['pseudopotentials'] = bigdct.pop('pseudopotentials', {}) + # Check for unexpected blocks for block in bigdct: raise ValueError(f'Unrecognized block "{block}" in the json input file')