Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove non-robust "algebraic parsing" #187

Merged
merged 7 commits into from
Feb 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/koopmans/settings/_koopmans_cp.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,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
Expand Down
1 change: 0 additions & 1 deletion src/koopmans/settings/_koopmans_ham.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion src/koopmans/settings/_koopmans_screen.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion src/koopmans/settings/_pw.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 0 additions & 1 deletion src/koopmans/settings/_pw2wannier.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions src/koopmans/settings/_ui.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
64 changes: 10 additions & 54 deletions src/koopmans/settings/_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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):
Expand Down
1 change: 0 additions & 1 deletion src/koopmans/settings/_wann2kc.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion src/koopmans/settings/_wann2kcp.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion src/koopmans/settings/_wannier90.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion src/koopmans/settings/_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')),
Expand Down
15 changes: 10 additions & 5 deletions src/koopmans/workflows/_workflow.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
"""
Generic workflow object for koopmans

Written by Edward Linscott Oct 2020

Converted workflows from functions to objects Nov 2020
"""

Expand Down Expand Up @@ -61,11 +63,13 @@ class Workflow(ABC):

r'''
Abstract base class that defines a Koopmans workflow

Parameters
----------

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
a dataclass defining the k-point sampling and paths
Expand Down Expand Up @@ -279,10 +283,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
Expand Down Expand Up @@ -1066,6 +1069,8 @@ def _fromjsondct(cls, bigdct: Dict[str, Any], override: Dict[str, Any] = {}):
# object but it is provided in the w90 subdictionary)
kwargs['projections'] = ProjectionBlocks.fromlist(w90_block_projs, w90_block_spins, atoms)

kwargs['pseudopotentials'] = bigdct.pop('pseudopotentials', {})

# Check for unexpected blocks
for block in bigdct:
raise ValueError(f'Unrecognized block "{block}" in the json input file')
Expand Down