From 9c07ed62763382e1e7e83941f6195bc2887d33f1 Mon Sep 17 00:00:00 2001 From: Peter N O Gillespie Date: Wed, 15 May 2024 15:35:26 +0000 Subject: [PATCH 1/4] `XspectraCrystalWorkChain`: Enable Symmetry Data Inputs Adds an input namespace for the `XspectraCrystalWorkChain` which allows the user to define the spacegroup and equivalent sites data for the incoming structure, thus instructing the WorkChain to generate structures and run calculations for only the sites specified. Changes: * Adds the `symmetry_data` input namespace to `XspectraCrystalWorkChain`, which the `WorkChain` will use to generate structures and set the list of polarisation vectors to calculate. * Adds input validation steps for the symmetry data to check for required information and for entries which may cause a crash, though does not check for issues beyond this in order to maximise flexibility of use. * Fixes an oversight in `get_xspectra_structures` where the `supercell` entry was not returned to the outputs when external symmetry data were provided by the user. --- .../functions/get_xspectra_structures.py | 1 + .../workflows/xspectra/crystal.py | 71 +++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/src/aiida_quantumespresso/workflows/functions/get_xspectra_structures.py b/src/aiida_quantumespresso/workflows/functions/get_xspectra_structures.py index 369bfa45..7dbcbaf0 100644 --- a/src/aiida_quantumespresso/workflows/functions/get_xspectra_structures.py +++ b/src/aiida_quantumespresso/workflows/functions/get_xspectra_structures.py @@ -360,6 +360,7 @@ def get_xspectra_structures(structure, **kwargs): # pylint: disable=too-many-st new_supercell = get_supercell_result['new_supercell'] output_params['supercell_factors'] = multiples + result['supercell'] = new_supercell output_params['supercell_num_sites'] = len(new_supercell.sites) output_params['supercell_cell_matrix'] = new_supercell.cell output_params['supercell_cell_lengths'] = new_supercell.cell_lengths diff --git a/src/aiida_quantumespresso/workflows/xspectra/crystal.py b/src/aiida_quantumespresso/workflows/xspectra/crystal.py index 1d56a50f..e222c982 100644 --- a/src/aiida_quantumespresso/workflows/xspectra/crystal.py +++ b/src/aiida_quantumespresso/workflows/xspectra/crystal.py @@ -173,6 +173,19 @@ def define(cls, spec): help=('Input namespace to provide core wavefunction inputs for each element. Must follow the format: ' '``core_wfc_data__{symbol} = {node}``') ) + spec.input_namespace( + 'symmetry_data', + valid_type=(orm.Dict, orm.Int), + dynamic=True, + required=False, + help=( + 'Input namespace to define equivalent sites and spacegroup number for the system. If defined, will ' + + 'skip symmetry analysis and structure standardization. Use *only* if symmetry data are known' + + 'for certain. Requires ``spacegroup_number`` (Int) and ``equivalent_sites_data`` (Dict) to be' + + 'defined separately. All keys in `equivalent_sites_data` must be formatted as "site_".' + + 'See docstring of `get_xspectra_structures` for more information about inputs.' + ) + ) spec.inputs.validator = cls.validate_inputs spec.outline( cls.setup, @@ -369,6 +382,7 @@ def get_builder_from_protocol( # pylint: disable=too-many-statements return builder + # pylint: disable=too-many-statements @staticmethod def validate_inputs(inputs, _): """Validate the inputs before launching the WorkChain.""" @@ -429,7 +443,58 @@ def validate_inputs(inputs, _): 'any wavefunction data.' ) + if 'symmetry_data' in inputs: + spacegroup_number = inputs['symmetry_data']['spacegroup_number'].value + equivalent_sites_data = inputs['symmetry_data']['equivalent_sites_data'].get_dict() + if spacegroup_number <= 0 or spacegroup_number >= 231: + raise ValidationError( + f'Input spacegroup number ({spacegroup_number}) outside of valid range (1-230).' + ) + + input_elements = [] + required_keys = sorted(['symbol', 'multiplicity', 'kind_name', 'site_index']) + invalid_entries = [] + # We check three things here: (1) are there any site indices which are outside of the possible + # range of site indices (2) do we have all the required keys for each entry, + # and (3) is there a mismatch between `absorbing_elements_list` and the elements specified + # in the entries of `equivalent_sites_data`. These checks are intended only to avoid a crash. + # We assume otherwise that the user knows what they're doing and has set everything else + # to their preferences correctly. + for site_label, value in equivalent_sites_data.items(): + required_keys_found = [] + + if value['site_index'] < 0: + raise ValidationError( + f'The site index for {site_label} ({value["site_index"]}) is below the range of ' + + f'sites within the structure (0-{len(structure.sites) -1}).' + ) + if value['site_index'] >= len(structure.sites): + raise ValidationError( + f'The site index for {site_label} ({value["site_index"]}) is above the range of ' + + f'sites within the structure (0-{len(structure.sites) -1}).' + ) + for key in value: + if key in required_keys: + required_keys_found.append(key) + if sorted(required_keys_found) != required_keys: + invalid_entries.append(site_label) + elif value['symbol'] not in input_elements: + input_elements.append(value['symbol']) + + if len(invalid_entries) != 0: + raise ValidationError( + f'The required keys ({required_keys}) were not found in the following entries: {invalid_entries}' + ) + + sorted_input_elements = sorted(input_elements) + if sorted_input_elements != absorbing_elements_list: + raise ValidationError( + f'Elements defined for sites in `equivalent_sites_data` ({sorted_input_elements}) do not match the' + + f'list of absorbing elements ({absorbing_elements_list})' + ) + + # pylint: enable=too-many-statements def setup(self): """Set required context variables.""" if 'core_wfc_data' in self.inputs.keys(): @@ -489,6 +554,12 @@ def get_xspectra_structures(self): if 'spglib_settings' in self.inputs: inputs['spglib_settings'] = self.inputs.spglib_settings + if 'symmetry_data' in self.inputs: + inputs['parse_symmetry'] = orm.Bool(False) + input_sym_data = self.inputs.symmetry_data + inputs['equivalent_sites_data'] = input_sym_data['equivalent_sites_data'] + inputs['spacegroup_number'] = input_sym_data['spacegroup_number'] + if 'relax' in self.inputs: result = get_xspectra_structures(self.ctx.optimized_structure, **inputs) else: From 7b2ffb04a2f11b55e1bee5d9f4e9eb1fc7132795 Mon Sep 17 00:00:00 2001 From: Peter N O Gillespie Date: Thu, 23 May 2024 11:51:34 +0000 Subject: [PATCH 2/4] `XspectraCrystalWorkChain`: Requested Changes 1 Changes requested for PR #1028: * Small refactor of input validation code to condense checks into fewer lines. * Re-arranged checks to ensure that `site_index` validation occurs *after* `required_keys` are checked. Previous behaviour would not indicate which entry/entries were missing the `site_index` * Fixed a small formatting error in the error message for mismatch in absorbing elements. --- .../workflows/xspectra/crystal.py | 27 ++++++++----------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/src/aiida_quantumespresso/workflows/xspectra/crystal.py b/src/aiida_quantumespresso/workflows/xspectra/crystal.py index e222c982..bfa01326 100644 --- a/src/aiida_quantumespresso/workflows/xspectra/crystal.py +++ b/src/aiida_quantumespresso/workflows/xspectra/crystal.py @@ -461,26 +461,21 @@ def validate_inputs(inputs, _): # We assume otherwise that the user knows what they're doing and has set everything else # to their preferences correctly. for site_label, value in equivalent_sites_data.items(): - required_keys_found = [] + entry_invalid = False - if value['site_index'] < 0: - raise ValidationError( - f'The site index for {site_label} ({value["site_index"]}) is below the range of ' - + f'sites within the structure (0-{len(structure.sites) -1}).' - ) - if value['site_index'] >= len(structure.sites): - raise ValidationError( - f'The site index for {site_label} ({value["site_index"]}) is above the range of ' - + f'sites within the structure (0-{len(structure.sites) -1}).' - ) - for key in value: - if key in required_keys: - required_keys_found.append(key) - if sorted(required_keys_found) != required_keys: + if not set(required_keys).issubset(set(value.keys())) : invalid_entries.append(site_label) + entry_invalid = True elif value['symbol'] not in input_elements: input_elements.append(value['symbol']) + if not entry_invalid: + if value['site_index'] < 0 or value['site_index'] >= len(structure.sites): + raise ValidationError( + f'The site index for {site_label} ({value["site_index"]}) is outside the range of ' + + f'sites within the structure (0-{len(structure.sites) -1}).' + ) + if len(invalid_entries) != 0: raise ValidationError( f'The required keys ({required_keys}) were not found in the following entries: {invalid_entries}' @@ -490,7 +485,7 @@ def validate_inputs(inputs, _): if sorted_input_elements != absorbing_elements_list: raise ValidationError( f'Elements defined for sites in `equivalent_sites_data` ({sorted_input_elements}) do not match the' - + f'list of absorbing elements ({absorbing_elements_list})' + + f' list of absorbing elements ({absorbing_elements_list})' ) From 7e5f96561f6953dfddf9defb5d876c3400711b04 Mon Sep 17 00:00:00 2001 From: Peter Gillespie <55498719+PNOGillespie@users.noreply.github.com> Date: Mon, 27 May 2024 09:48:01 +0200 Subject: [PATCH 3/4] `XspectraCrystalWorkChain`: Simplify Validation Check Logic Co-authored-by: Xing Wang --- src/aiida_quantumespresso/workflows/xspectra/crystal.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/aiida_quantumespresso/workflows/xspectra/crystal.py b/src/aiida_quantumespresso/workflows/xspectra/crystal.py index bfa01326..d440afff 100644 --- a/src/aiida_quantumespresso/workflows/xspectra/crystal.py +++ b/src/aiida_quantumespresso/workflows/xspectra/crystal.py @@ -461,15 +461,10 @@ def validate_inputs(inputs, _): # We assume otherwise that the user knows what they're doing and has set everything else # to their preferences correctly. for site_label, value in equivalent_sites_data.items(): - entry_invalid = False - if not set(required_keys).issubset(set(value.keys())) : invalid_entries.append(site_label) - entry_invalid = True elif value['symbol'] not in input_elements: input_elements.append(value['symbol']) - - if not entry_invalid: if value['site_index'] < 0 or value['site_index'] >= len(structure.sites): raise ValidationError( f'The site index for {site_label} ({value["site_index"]}) is outside the range of ' From 28d10f50ee69018a29528c0a9142a20eeb221350 Mon Sep 17 00:00:00 2001 From: Peter N O Gillespie Date: Wed, 10 Jul 2024 14:59:01 +0000 Subject: [PATCH 4/4] `XspectraCrystalWorkChain`: Correct Error Handling Changes requested for PR #1028 (2) Corrects error handling behaviour for `validate_inputs` to use `return` rather than `raise`. Also fixes minor formatting errors in `symmetry_data` `help` entry and removes unnecessary uses of `+`. --- .../workflows/xspectra/crystal.py | 70 +++++++++---------- 1 file changed, 33 insertions(+), 37 deletions(-) diff --git a/src/aiida_quantumespresso/workflows/xspectra/crystal.py b/src/aiida_quantumespresso/workflows/xspectra/crystal.py index d440afff..76c866fc 100644 --- a/src/aiida_quantumespresso/workflows/xspectra/crystal.py +++ b/src/aiida_quantumespresso/workflows/xspectra/crystal.py @@ -4,7 +4,7 @@ Uses QuantumESPRESSO pw.x and xspectra.x. """ from aiida import orm -from aiida.common import AttributeDict, ValidationError +from aiida.common import AttributeDict from aiida.engine import ToContext, WorkChain, if_ from aiida.orm import UpfData as aiida_core_upf from aiida.plugins import CalculationFactory, DataFactory, WorkflowFactory @@ -180,10 +180,10 @@ def define(cls, spec): required=False, help=( 'Input namespace to define equivalent sites and spacegroup number for the system. If defined, will ' - + 'skip symmetry analysis and structure standardization. Use *only* if symmetry data are known' - + 'for certain. Requires ``spacegroup_number`` (Int) and ``equivalent_sites_data`` (Dict) to be' - + 'defined separately. All keys in `equivalent_sites_data` must be formatted as "site_".' - + 'See docstring of `get_xspectra_structures` for more information about inputs.' + 'skip symmetry analysis and structure standardization. Use *only* if symmetry data are known ' + 'for certain. Requires ``spacegroup_number`` (Int) and ``equivalent_sites_data`` (Dict) to be ' + 'defined separately. All keys in `equivalent_sites_data` must be formatted as "site_". ' + 'See docstring of `get_xspectra_structures` for more information about inputs.' ) ) spec.inputs.validator = cls.validate_inputs @@ -382,9 +382,8 @@ def get_builder_from_protocol( # pylint: disable=too-many-statements return builder - # pylint: disable=too-many-statements @staticmethod - def validate_inputs(inputs, _): + def validate_inputs(inputs, _): # pylint: disable=too-many-return-statements """Validate the inputs before launching the WorkChain.""" structure = inputs['structure'] kinds_present = [kind.name for kind in structure.kinds] @@ -396,58 +395,57 @@ def validate_inputs(inputs, _): if element not in elements_present: extra_elements.append(element) if len(extra_elements) > 0: - raise ValidationError( + return ( f'Some elements in ``elements_list`` {extra_elements} do not exist in the' f' structure provided {elements_present}.' ) abs_atom_marker = inputs['abs_atom_marker'].value if abs_atom_marker in kinds_present: - raise ValidationError( + return ( f'The marker given for the absorbing atom ("{abs_atom_marker}") matches an existing Kind in the ' f'input structure ({kinds_present}).' ) if not inputs['core']['get_powder_spectrum'].value: - raise ValidationError( + return ( 'The ``get_powder_spectrum`` input for the XspectraCoreWorkChain namespace must be ``True``.' ) if 'upf2plotcore_code' not in inputs and 'core_wfc_data' not in inputs: - raise ValidationError( + return ( 'Neither a ``Code`` node for upf2plotcore.sh or a set of ``core_wfc_data`` were provided.' ) if 'core_wfc_data' in inputs: core_wfc_data_list = sorted(inputs['core_wfc_data'].keys()) if core_wfc_data_list != absorbing_elements_list: - raise ValidationError( + return ( f'The ``core_wfc_data`` provided ({core_wfc_data_list}) does not match the list of' f' absorbing elements ({absorbing_elements_list})' ) - else: - empty_core_wfc_data = [] - for key, value in inputs['core_wfc_data'].items(): - header_line = value.get_content()[:40] - try: - num_core_states = int(header_line.split(' ')[5]) - except Exception as exc: - raise ValidationError( - 'The core wavefunction data file is not of the correct format' - ) from exc - if num_core_states == 0: - empty_core_wfc_data.append(key) - if len(empty_core_wfc_data) > 0: - raise ValidationError( - f'The ``core_wfc_data`` provided for elements {empty_core_wfc_data} do not contain ' - 'any wavefunction data.' - ) + empty_core_wfc_data = [] + for key, value in inputs['core_wfc_data'].items(): + header_line = value.get_content()[:40] + try: + num_core_states = int(header_line.split(' ')[5]) + except: # pylint: disable=bare-except + return ( + 'The core wavefunction data file is not of the correct format' + ) # pylint: enable=bare-except + if num_core_states == 0: + empty_core_wfc_data.append(key) + if len(empty_core_wfc_data) > 0: + return ( + f'The ``core_wfc_data`` provided for elements {empty_core_wfc_data} do not contain ' + 'any wavefunction data.' + ) if 'symmetry_data' in inputs: spacegroup_number = inputs['symmetry_data']['spacegroup_number'].value equivalent_sites_data = inputs['symmetry_data']['equivalent_sites_data'].get_dict() if spacegroup_number <= 0 or spacegroup_number >= 231: - raise ValidationError( + return ( f'Input spacegroup number ({spacegroup_number}) outside of valid range (1-230).' ) @@ -466,25 +464,23 @@ def validate_inputs(inputs, _): elif value['symbol'] not in input_elements: input_elements.append(value['symbol']) if value['site_index'] < 0 or value['site_index'] >= len(structure.sites): - raise ValidationError( + return ( f'The site index for {site_label} ({value["site_index"]}) is outside the range of ' + f'sites within the structure (0-{len(structure.sites) -1}).' ) if len(invalid_entries) != 0: - raise ValidationError( + return ( f'The required keys ({required_keys}) were not found in the following entries: {invalid_entries}' ) sorted_input_elements = sorted(input_elements) if sorted_input_elements != absorbing_elements_list: - raise ValidationError( - f'Elements defined for sites in `equivalent_sites_data` ({sorted_input_elements}) do not match the' - + f' list of absorbing elements ({absorbing_elements_list})' - ) + return (f'Elements defined for sites in `equivalent_sites_data` ({sorted_input_elements}) ' + f'do not match the list of absorbing elements ({absorbing_elements_list})') - # pylint: enable=too-many-statements + # pylint: enable=too-many-return-statements def setup(self): """Set required context variables.""" if 'core_wfc_data' in self.inputs.keys():