diff --git a/applications/io_demo/example/iodef.xml b/applications/io_demo/example/iodef.xml index c4deecb41..8f84f23f2 100644 --- a/applications/io_demo/example/iodef.xml +++ b/applications/io_demo/example/iodef.xml @@ -3,9 +3,9 @@ - - - + + + - \ No newline at end of file + diff --git a/applications/lbc_demo/example/iodef.xml b/applications/lbc_demo/example/iodef.xml index 47855ec1f..a88aace68 100644 --- a/applications/lbc_demo/example/iodef.xml +++ b/applications/lbc_demo/example/iodef.xml @@ -2,9 +2,9 @@ - - - + + + @@ -24,9 +24,9 @@ - - - + + + diff --git a/applications/lbc_demo/example/metadata b/applications/lbc_demo/example/metadata new file mode 120000 index 000000000..56e175b1c --- /dev/null +++ b/applications/lbc_demo/example/metadata @@ -0,0 +1 @@ +../metadata/ \ No newline at end of file diff --git a/applications/lbc_demo/metadata/field_dictionary.xml b/applications/lbc_demo/metadata/field_dictionary.xml index 103f7e3f5..21a9c8d57 100644 --- a/applications/lbc_demo/metadata/field_dictionary.xml +++ b/applications/lbc_demo/metadata/field_dictionary.xml @@ -19,4 +19,4 @@ - + diff --git a/applications/simple_diffusion/example/iodef.xml b/applications/simple_diffusion/example/iodef.xml index 39033c256..51ce82a2e 100644 --- a/applications/simple_diffusion/example/iodef.xml +++ b/applications/simple_diffusion/example/iodef.xml @@ -3,9 +3,9 @@ - - - + + + - \ No newline at end of file + diff --git a/documentation/source/how_to_use_it/io/io_standards.rst b/documentation/source/how_to_use_it/io/io_standards.rst index 866490cc1..9c6609acc 100644 --- a/documentation/source/how_to_use_it/io/io_standards.rst +++ b/documentation/source/how_to_use_it/io/io_standards.rst @@ -20,16 +20,22 @@ Standards for XIOS Iodef Files section of every top-level iodef file. #. Non-global file settings are to be specified at the level of individual files. + #. ``id`` values shall be unique for a given ``XIOS`` ``XML`` element + type. **Rationale** .. warning:: - When XIOS aggregates file definitions - possibly spread across - several XML files -, the last set of attributes processed "wins", - in the sense that all the others are silently ignored. + When XIOS aggregates definitions - possibly spread across + several XML files -, then attributes are overriden by defined elements + that use a ``_ref``. - This can create considerable confusion and frustration for users. + The overriding operates in a chain, so users may need to act cautiously + to understand attribute inheritance and overrides. + + This is why reuse of ``id`` values for different elements is problematic, + not allowed, and tested by a code inspection rule for ``field`` elements. With the current layout of the LFRic top-level iodef files, the last set of attributes to be processed are the ones in the local file diff --git a/documentation/source/how_to_use_it/io/prognostics.rst b/documentation/source/how_to_use_it/io/prognostics.rst index 7d0a1de36..620af65ea 100644 --- a/documentation/source/how_to_use_it/io/prognostics.rst +++ b/documentation/source/how_to_use_it/io/prognostics.rst @@ -286,3 +286,12 @@ to other model requirements including models that do not use XIOS. checkpoint flag setting, to the ``checkpoint_fields`` field collection. Additionally, like all other prognostic fields, it is added to the ``depository`` field collection. + + ``iodef.xml`` files may reference further ``.xml`` files using the + ``src=""`` attribute. File paths in the source tree are maintained + using symbolic links, which are dereferenced by ``rose-app.conf`` + to enable running tests. All links must resolve within the source, + and validation is implemented in a test called ``validate_iodef``. + Only the shared XIOS runtime configuration files within ``/etc`` + are updated to use environment variables, these should not be used + within other ``.xml`` fragments within the ``lfric_core`` code base. diff --git a/etc/xios.xml b/etc/xios.xml new file mode 100644 index 000000000..07879f5e6 --- /dev/null +++ b/etc/xios.xml @@ -0,0 +1,14 @@ + + + + performance + + + + $XIOS_SERVER_MODE_XML + 50 + true + + + + diff --git a/etc/xios_coupled.xml b/etc/xios_coupled.xml new file mode 100644 index 000000000..4d6fd18d3 --- /dev/null +++ b/etc/xios_coupled.xml @@ -0,0 +1,16 @@ + + + + performance + + + + $XIOS_SERVER_MODE_XML + 50 + true + true + lfric + + + + diff --git a/rose-stem/app/canned_test/rose-app.conf b/rose-stem/app/canned_test/rose-app.conf index 2934ac1b2..28058f0b0 100644 --- a/rose-stem/app/canned_test/rose-app.conf +++ b/rose-stem/app/canned_test/rose-app.conf @@ -1,6 +1,6 @@ [command] -aprun=eval $CANNED_PRESCRIPT ; $CORE_ROOT_DIR/bin/tweak_iodef ; aprun -cc depth -n ${PROCESSES:-1} -d $OMP_NUM_THREADS -j $HYPERTHREADS ${BIN_DIR}/${APPLICATION} ${CONFIGURATION_FILE} +aprun=eval $CANNED_PRESCRIPT ; aprun -cc depth -n ${PROCESSES:-1} -d $OMP_NUM_THREADS -j $HYPERTHREADS ${BIN_DIR}/${APPLICATION} ${CONFIGURATION_FILE} default=eval $CANNED_PRESCRIPT ; $CORE_ROOT_DIR/bin/tweak_iodef ; ${BIN_DIR}/${APPLICATION} ${CONFIGURATION_FILE} -mpiexec=eval $CANNED_PRESCRIPT ; $CORE_ROOT_DIR/bin/tweak_iodef ; mpiexec -n ${PROCESSES:-1} ${BIN_DIR}/${APPLICATION} ${CONFIGURATION_FILE} -mpiexec_cpl=eval $CANNED_PRESCRIPT ; $CORE_ROOT_DIR/bin/tweak_iodef ; mpiexec -n ${PROCESSES:-1} ${BIN_DIR}/${APPLICATION} ${CONFIGURATION_FILE1} -c lfric_o : -n ${PROCESSES:-1} ${BIN_DIR}/${APPLICATION} ${CONFIGURATION_FILE2} -c lfric_i -srun=eval $CANNED_PRESCRIPT ; $CORE_ROOT_DIR/bin/tweak_iodef ; srun --ntasks=${PROCESSES:-1} ${BIN_DIR}/${APPLICATION} ${CONFIGURATION_FILE} +mpiexec=eval $CANNED_PRESCRIPT ; mpiexec -n ${PROCESSES:-1} ${BIN_DIR}/${APPLICATION} ${CONFIGURATION_FILE} +mpiexec_cpl=eval $CANNED_PRESCRIPT ; mpiexec -n ${PROCESSES:-1} ${BIN_DIR}/${APPLICATION} ${CONFIGURATION_FILE1} -c lfric_o : -n ${PROCESSES:-1} ${BIN_DIR}/${APPLICATION} ${CONFIGURATION_FILE2} -c lfric_i +srun=eval $CANNED_PRESCRIPT ; srun --ntasks=${PROCESSES:-1} ${BIN_DIR}/${APPLICATION} ${CONFIGURATION_FILE} diff --git a/rose-stem/app/coupled/file/etc b/rose-stem/app/coupled/file/etc new file mode 120000 index 000000000..07008abc1 --- /dev/null +++ b/rose-stem/app/coupled/file/etc @@ -0,0 +1 @@ +../../../../etc/ \ No newline at end of file diff --git a/rose-stem/app/coupled/file/iodef.xml b/rose-stem/app/coupled/file/iodef.xml index d32b67fb6..126cc239f 100644 --- a/rose-stem/app/coupled/file/iodef.xml +++ b/rose-stem/app/coupled/file/iodef.xml @@ -55,20 +55,6 @@ - - - - performance - - - - false - 50 - true - true - lfric - - - + diff --git a/rose-stem/app/coupled/rose-app.conf b/rose-stem/app/coupled/rose-app.conf index 2b402b3c8..c97bf550c 100644 --- a/rose-stem/app/coupled/rose-app.conf +++ b/rose-stem/app/coupled/rose-app.conf @@ -1,7 +1,9 @@ meta=lfric-coupled/vn3.2 [command] -default=rose app-run -C $CYLC_WORKFLOW_RUN_DIR/app/coupled -O lam -O LAM50x50-2x2 --install-only +default=rose env-cat -o etc/updated_xios.xml etc/xios_coupled.xml + =mv etc/updated_xios.xml etc/xios_coupled.xml + =rose app-run -C $CYLC_WORKFLOW_RUN_DIR/app/coupled -O lam -O LAM50x50-2x2 --install-only =rose app-run -C $CYLC_WORKFLOW_RUN_DIR/app/coupled -O glo -O C12 --install-only =$LAUNCH_SCRIPT/cpl-launch-exe @@ -27,6 +29,9 @@ source=namelist:base_mesh = namelist:time = namelist:timestepping +[file:etc] +source=${CYLC_WORKFLOW_SHARE_DIR}/source/lfric_core/etc + [namelist:base_mesh] f_lat_deg=45.0 file_prefix='mesh' diff --git a/rose-stem/app/io_demo/file/etc b/rose-stem/app/io_demo/file/etc new file mode 120000 index 000000000..07008abc1 --- /dev/null +++ b/rose-stem/app/io_demo/file/etc @@ -0,0 +1 @@ +../../../../etc/ \ No newline at end of file diff --git a/rose-stem/app/io_demo/file/iodef.xml b/rose-stem/app/io_demo/file/iodef.xml index 442c373ce..0f3626f95 100644 --- a/rose-stem/app/io_demo/file/iodef.xml +++ b/rose-stem/app/io_demo/file/iodef.xml @@ -3,11 +3,11 @@ - + - + - + - - - - performance - - - - true - 50 - true - - - + diff --git a/rose-stem/app/io_demo/file/iodef_temporal.xml b/rose-stem/app/io_demo/file/iodef_temporal.xml index 77e38b572..16dd8919e 100644 --- a/rose-stem/app/io_demo/file/iodef_temporal.xml +++ b/rose-stem/app/io_demo/file/iodef_temporal.xml @@ -3,9 +3,9 @@ - - - + + + - - - - performance - - - - true - 50 - true - - - + diff --git a/rose-stem/app/io_demo/file/metadata b/rose-stem/app/io_demo/file/metadata new file mode 120000 index 000000000..9b32a1c6e --- /dev/null +++ b/rose-stem/app/io_demo/file/metadata @@ -0,0 +1 @@ +../../../../applications/io_demo/metadata/ \ No newline at end of file diff --git a/rose-stem/app/io_demo/file/temporal_context.xml b/rose-stem/app/io_demo/file/temporal_context.xml index af51348d0..fb7e2aee6 100644 --- a/rose-stem/app/io_demo/file/temporal_context.xml +++ b/rose-stem/app/io_demo/file/temporal_context.xml @@ -1,8 +1,8 @@ - - + + - - - + + + @@ -23,9 +23,9 @@ - - - + + + @@ -43,18 +43,6 @@ - - - - performance - - - - true - 50 - true - - - + diff --git a/rose-stem/app/lbc_demo/file/metadata b/rose-stem/app/lbc_demo/file/metadata new file mode 120000 index 000000000..648c201f7 --- /dev/null +++ b/rose-stem/app/lbc_demo/file/metadata @@ -0,0 +1 @@ +../../../../applications/lbc_demo/metadata/ \ No newline at end of file diff --git a/rose-stem/app/lbc_demo/rose-app.conf b/rose-stem/app/lbc_demo/rose-app.conf index 69ad1de7e..bd1c70138 100644 --- a/rose-stem/app/lbc_demo/rose-app.conf +++ b/rose-stem/app/lbc_demo/rose-app.conf @@ -1,7 +1,8 @@ meta=lfric-lbc_demo/vn3.2 [command] -default=$CORE_ROOT_DIR/bin/tweak_iodef ; \ +default=rose env-cat -o etc/updated_xios.xml etc/xios.xml + =mv etc/updated_xios.xml etc/xios.xml =$LAUNCH_SCRIPT/launch-exe [env] @@ -26,6 +27,12 @@ source=namelist:base_mesh = namelist:time = namelist:timestepping +[file:etc] +source=${CYLC_WORKFLOW_SHARE_DIR}/source/lfric_core/etc + +[file:metadata] +source=$METADATA + [namelist:base_mesh] f_lat_deg=45.0 file_prefix='' diff --git a/rose-stem/app/simple_diffusion/file/etc b/rose-stem/app/simple_diffusion/file/etc new file mode 120000 index 000000000..07008abc1 --- /dev/null +++ b/rose-stem/app/simple_diffusion/file/etc @@ -0,0 +1 @@ +../../../../etc/ \ No newline at end of file diff --git a/rose-stem/app/simple_diffusion/file/iodef.xml b/rose-stem/app/simple_diffusion/file/iodef.xml index d3db83f7a..5192970cc 100644 --- a/rose-stem/app/simple_diffusion/file/iodef.xml +++ b/rose-stem/app/simple_diffusion/file/iodef.xml @@ -3,11 +3,11 @@ - + - + - + - - - - performance - - - - true - 50 - true - - - + diff --git a/rose-stem/app/simple_diffusion/file/metadata b/rose-stem/app/simple_diffusion/file/metadata new file mode 120000 index 000000000..d151871be --- /dev/null +++ b/rose-stem/app/simple_diffusion/file/metadata @@ -0,0 +1 @@ +../../../../applications/simple_diffusion/metadata/ \ No newline at end of file diff --git a/rose-stem/app/simple_diffusion/rose-app.conf b/rose-stem/app/simple_diffusion/rose-app.conf index c4ad299fc..863a1c73f 100644 --- a/rose-stem/app/simple_diffusion/rose-app.conf +++ b/rose-stem/app/simple_diffusion/rose-app.conf @@ -1,7 +1,8 @@ meta=lfric-simple_diffusion/vn3.2 [command] -default=$CORE_ROOT_DIR/bin/tweak_iodef ; \ +default=rose env-cat -o etc/updated_xios.xml etc/xios.xml + =mv etc/updated_xios.xml etc/xios.xml =$LAUNCH_SCRIPT/launch-exe [env] @@ -26,6 +27,12 @@ source=namelist:base_mesh = namelist:time = namelist:timestepping +[file:etc] +source=${CYLC_WORKFLOW_SHARE_DIR}/source/lfric_core/etc + +[file:metadata] +source=${CYLC_WORKFLOW_SHARE_DIR}/source/lfric_core/applications/simple_diffusion/metadata + [namelist:base_mesh] f_lat_deg=45.0 file_prefix='' diff --git a/rose-stem/app/skeleton/file/etc b/rose-stem/app/skeleton/file/etc new file mode 120000 index 000000000..07008abc1 --- /dev/null +++ b/rose-stem/app/skeleton/file/etc @@ -0,0 +1 @@ +../../../../etc/ \ No newline at end of file diff --git a/rose-stem/app/skeleton/file/iodef.xml b/rose-stem/app/skeleton/file/iodef.xml index 786cfffa0..0c590d865 100644 --- a/rose-stem/app/skeleton/file/iodef.xml +++ b/rose-stem/app/skeleton/file/iodef.xml @@ -55,18 +55,6 @@ - - - - performance - - - - true - 50 - true - - - + diff --git a/rose-stem/app/skeleton/rose-app.conf b/rose-stem/app/skeleton/rose-app.conf index 41784ce12..6e854e6a0 100644 --- a/rose-stem/app/skeleton/rose-app.conf +++ b/rose-stem/app/skeleton/rose-app.conf @@ -1,7 +1,9 @@ meta=lfric-skeleton/vn3.2 [command] -default=$LAUNCH_SCRIPT/launch-exe +default=rose env-cat -o etc/updated_xios.xml etc/xios.xml + =mv etc/updated_xios.xml etc/xios.xml + =$LAUNCH_SCRIPT/launch-exe [env] EXEC_NAME=skeleton @@ -25,6 +27,9 @@ source=namelist:base_mesh = namelist:time = namelist:timestepping +[file:etc] +source=${CYLC_WORKFLOW_SHARE_DIR}/source/lfric_core/etc + [namelist:base_mesh] f_lat_deg=45.0 file_prefix='' diff --git a/rose-stem/app/validate_iodef/bin/test_validate_iodef_xml.py b/rose-stem/app/validate_iodef/bin/test_validate_iodef_xml.py new file mode 100644 index 000000000..e2189d18a --- /dev/null +++ b/rose-stem/app/validate_iodef/bin/test_validate_iodef_xml.py @@ -0,0 +1,103 @@ +#!/usr/bin/env python3 +############################################################################## +# (c) Crown copyright Met Office. All rights reserved. +# The file LICENCE, distributed with this code, contains details of the terms +# under which the code may be used. +############################################################################## +""" +XIOS Iodef.xml metadata consistency validation tests. +""" + +import glob +import os +import re +import xml.etree.ElementTree as ET + +import pytest + + +# security pattern to check whether `src` links are local and link to known +# controlled facets of the local source tree. Do not load from unknown sources. +# allows strings that start with: +# 'metadata/' etc/' './' '$SOURCE_ROOT/' +# followed by alphabetic characters only +security_re = re.compile(r'^(metadata/|etc/|\./|\$SOURCE_ROOT/)' + r'[a-zA-Z]+\w') + +def src_replace(parent, path): + """ + Recursively update input parent element, resolVing `src` content and + replacing this with parsed XML elements from referenced files. + """ + i = 0 + for elem in parent: + if elem.attrib.get('src'): + inf = os.path.join(path, elem.attrib['src']) + print(f'parsing: {inf}') + newelem = ET.parse(inf).getroot() + parent[i] = newelem + else: + src_replace(elem, path) + i += 1 + + +def load_source_xml(fname): + """ + Load a source XML file and recursively populated `src` links from + identified safe local paths. + """ + path = os.path.dirname(fname) + load_tree = ET.parse(fname) + load_root = load_tree.getroot() + + # Validate that external elements, defined using `src=` are only accessing + # known internal paths to minimise rish from "XML external entity attack" + for elem in load_root.findall('.//*[@src]'): + if security_re.match(elem.attrib['src']) is None: + raise ValueError('only `src` attributes from local, `metadata/`, ' + '`etc/` or `$SOURCE_ROOT/`paths are supported, not ' + f'{elem.attrib["src"]}') + src_replace(load_root, path) + return load_tree + +# Generator for the pytest parametrize fixture. +root_dir = os.environ.get('SOURCE_PATH', '') +print(f'root_dir = {root_dir}') +iodef_likes = [] +for iodef_like in glob.glob('**/iodef*.xml', + root_dir=root_dir, + recursive=True): + infile = os.path.join(root_dir, iodef_like) + tree = load_source_xml(infile) + root = tree.getroot() + iodef_likes.append((root, infile)) + +if len(iodef_likes) == 0: + raise ValueError('No `iodef*.xml` files found.') + +@pytest.mark.parametrize("aroot, test_file", iodef_likes) +def test_unique_field_ids_within_context(aroot, test_file): + """ + Pytest test to verify that `field` element `id`s are unique within + a given XIOS context. + """ + print(f'validating XIOS XML for {aroot} ...') + + for context in aroot.findall('.//context'): + field_ids = set() + for elem in context.findall('.//field[@id]'): + eid = elem.attrib.get('id') + if eid is not None: + err_str = (f'Within {test_file}\n:' + '\tThe `field` element:\n' + f'{elem.attrib}\n' + f'\thas an `id`: "{eid}" ' + 'which is already defined for an existing field.\n' + '\tXIOS will not distinguish betwen these fields, ' + 'operations using this id will affect all fields ' + 'with this `id` leading to indeterminate behaviour') + assert eid not in field_ids, err_str + field_ids.add(eid) + +if __name__ == '__main__': + pytest.main() diff --git a/rose-stem/app/validate_iodef/rose-app.conf b/rose-stem/app/validate_iodef/rose-app.conf new file mode 100644 index 000000000..44c82d8e0 --- /dev/null +++ b/rose-stem/app/validate_iodef/rose-app.conf @@ -0,0 +1,2 @@ +[command] +default=pytest $CYLC_WORKFLOW_RUN_DIR/app/validate_iodef/bin/test_validate_iodef_xml.py diff --git a/rose-stem/site/meto/groups.cylc b/rose-stem/site/meto/groups.cylc index 62646ad80..fbfb9c2b1 100644 --- a/rose-stem/site/meto/groups.cylc +++ b/rose-stem/site/meto/groups.cylc @@ -14,6 +14,7 @@ "validate_rose_meta", "rose-stem_lint_checker", "global_variables_checker", + "validate_iodef", "python_unit_tests" ], "azspice_developer": [], diff --git a/rose-stem/templates/graph/populate_graph_scripts.cylc b/rose-stem/templates/graph/populate_graph_scripts.cylc index 2aa328729..10493f1d8 100644 --- a/rose-stem/templates/graph/populate_graph_scripts.cylc +++ b/rose-stem/templates/graph/populate_graph_scripts.cylc @@ -56,6 +56,7 @@ {# * validate_rose_meta #} {# * global_variables_checker #} {# * python_unit_tests #} + {# * validate_iodef #} {# * local_build_test #} {% do graph_sections.append([ diff --git a/rose-stem/templates/runtime/generate_runtime_application.cylc b/rose-stem/templates/runtime/generate_runtime_application.cylc index ce88cf3db..cfd90ad8a 100644 --- a/rose-stem/templates/runtime/generate_runtime_application.cylc +++ b/rose-stem/templates/runtime/generate_runtime_application.cylc @@ -34,8 +34,9 @@ {% set restart_stem_name = checkpoint_dir~"/restart_"~task_family~"-" %} {# Set the pre-script command for canned tests #} -{% set canned_prescript = "cp -r $SOURCE_DIRECTORY/"~task_values["example_dir"]~"/* "~ - "$CYLC_TASK_WORK_DIR" %} +{# Copy the source, dereferencing symbolic links #} +{% set canned_prescript = "cp -rL $SOURCE_DIRECTORY/"~task_values["example_dir"]~"/* "~ + "$CYLC_TASK_WORK_DIR "%} {# Set the post script command - should be a list of strings #} {% set post_script_commands = [ @@ -143,6 +144,7 @@ CHECKPOINT_STEM_FILE = {{restart_stem_name}} CANNED_PRESCRIPT = {{canned_prescript}} XIOS_SERVER_MODE = {{task_values["xios_server_mode"]}} + XIOS_SERVER_MODE_XML = {{(task_values["xios_server_mode"]).__str__().lower() }} XIOS_SERVER_RANKS = {{task_values["xios_server_ranks"]}} xios_nodes = {{task_values["xios_nodes"]}} mpi_parts_xios = {{task_values["mpi_parts_xios"]}} diff --git a/rose-stem/templates/runtime/generate_runtime_scripts.cylc b/rose-stem/templates/runtime/generate_runtime_scripts.cylc index 3c501f06f..0fb2081e9 100644 --- a/rose-stem/templates/runtime/generate_runtime_scripts.cylc +++ b/rose-stem/templates/runtime/generate_runtime_scripts.cylc @@ -56,6 +56,13 @@ inherit={{inherit.str|upper}} script="rose task-run --app-key=rose-stem_lint_check" +{% elif "validate_iodef" in task %} + + inherit={{inherit.str|upper}} + script="rose task-run --app-key=validate_iodef" + [[[environment]]] + SOURCE_PATH = $SOURCE_ROOT/lfric_core + {% elif "validate_rose_meta" in task %} inherit={{inherit.str|upper}}