Prefix all namelist group and option names for MPAS dycore #1285

kuanchihwang · 2025-01-30T00:29:09Z

When MPAS is used as a dynamical core in a host model (e.g., CAM/CAM-SIMA), it needs to share the namelist file with other model components. As a result, MPAS namelist groups and options may not be easily recognizable at first sight. The solution, which is implemented by this PR, is to add a unique identifier to all MPAS namelist group and option names.

The following transformations are performed for each MPAS namelist group and option name:

Leading config_ is removed recursively from the name. Case-insensitive.
Leading mpas_ is removed recursively from the name. Case-insensitive.
Prepend mpas_ to the name.

By doing so, it is now easier to distinguish MPAS namelist groups and options from host model ones. The possibility of name collisions with host model ones is also resolved once and for all. Note that only namelist I/O is affected. Internally, MPAS still refers to its namelist options by their original names due to compatibility reasons.

Compared to the implementation in CAM, this PR applies the "name mangling" logic in an algorithmic and predictable way. It works automatically even if there are additions, modifications, or deletions to MPAS namelist groups and options.

The "name mangling" logic is entirely guarded behind the MPAS_CAM_DYCORE macro. For stand-alone MPAS, its namelist groups and options keep their original names as in the registry. For CAM, it is not affected by this PR because it does not use the code generation functionality of MPAS for namelist reading at all. For CAM-SIMA, the Fortran include files generated by the parse utility stay the same. Its regression tests all pass, indicating identical model results to the previous baseline.

abishekg7 · 2025-02-07T17:19:28Z

Just started looking at this. One question I have is why the leading mpas_ and config_ strings need to be removed recursively and not just once.

kuanchihwang · 2025-02-07T21:04:07Z

Just started looking at this. One question I have is why the leading mpas_ and config_ strings need to be removed recursively and not just once.

Performing the removal operation only once would also work. However, the recursion should make it more robust in the unlikely case that a namelist option with duplicate prefixes is introduced to MPAS registry.

abishekg7 · 2025-02-07T22:16:53Z

Just started looking at this. One question I have is why the leading mpas_ and config_ strings need to be removed recursively and not just once.

Performing the removal operation only once would also work. However, the recursion should make it more robust in the unlikely case that a namelist option with duplicate prefixes is introduced to MPAS registry.

Thanks @kuanchihwang. The code itself looks fine, but I was wondering how to go about testing this implementation. You provided some CAM-SIMA build instructions earlier - can I use those?

kuanchihwang · 2025-02-07T23:22:10Z

@abishekg7

Sure! I will also provide the instructions here for clarity and convenience:

Output the changes in this PR as a patch by git format-patch (git diff also works).
Follow the steps below on Derecho:

git clone https://github.com/ESCOMP/CAM-SIMA.git
cd CAM-SIMA
git checkout development
./bin/git-fleximod update
# Replace the patch at "src/dynamics/mpas/assets/0001-Prefix-all-MPAS-namelist-group-and-option-names.patch" with the one that you generated in step 1. File name needs to end with "*.patch".
mkdir -pv ../PR1285 && cd ../PR1285

# Compile with GNU compilers
../CAM-SIMA/cime/scripts/create_newcase --compiler gnu --case PR1285-GNU --compset FKESSLER --project PROJECT_KEY --res mpasa480_mpasa480 --run-unsupported
cd PR1285-GNU
# Compile with Intel compilers
../CAM-SIMA/cime/scripts/create_newcase --compiler intel --case PR1285-Intel --compset FKESSLER --project PROJECT_KEY --res mpasa480_mpasa480 --run-unsupported
cd PR1285-Intel

./case.setup
./case.build

# Inspect the build artifacts of MPAS at:
#   "$SCRATCH/PR1285-GNU/bld/atm/obj/mpas" for GNU compilers
#   "$SCRATCH/PR1285-Intel/bld/atm/obj/mpas" for Intel compilers
#
# Specifically, inspect the "namelist_call.inc" and "namelist_defines.inc" include files at:
#   "$SCRATCH/PR1285-GNU/bld/atm/obj/mpas/core_atmosphere/inc" for GNU compilers
#   "$SCRATCH/PR1285-Intel/bld/atm/obj/mpas/core_atmosphere/inc" for Intel compilers
#
# You will find that MPAS namelist group and option names have been prefixed with "mpas_".

Similarly, this PR is also an attempt to upstream that particular patch so we do not have to maintain it downstream at CAM-SIMA. Thanks so much for your time!

abishekg7 · 2025-02-08T01:39:02Z

Thanks for the instructions @kuanchihwang . I follow them and inspected the namelist_defines.inc for both the builds - Looks good to me.

The only comment I have is re. the commit message. Could you reference (CAM/CAM-SIMA) in the title of the commit message and also the first line inside the message. i.e it ends with "When MPAS is used as a dynamical core". Could you add "in a host model (e.g., CAM/CAM-SIMA)" right after this, just like in the PR description.

kuanchihwang · 2025-02-13T22:24:17Z

@abishekg7

Thanks for the comments! I have updated the commit title and message to mention CAM/CAM-SIMA.

abishekg7

Looks good!

mgduda · 2025-02-19T23:06:47Z

src/tools/registry/gen_inc.c

@@ -699,6 +703,14 @@ int parse_namelist_records_from_registry(ezxml_t registry)/*{{{*/
 	const char *nmlrecname, *nmlrecindef, *nmlrecinsub;
 	const char *nmloptname, *nmlopttype, *nmloptval, *nmloptunits, *nmloptdesc, *nmloptposvals, *nmloptindef;

+#ifdef MPAS_CAM_DYCORE


I'm interested in discussing the idea further, but rather than adding #ifdef directives in many places throughout the code, what if we were to frame the transform_name function in a more general way, as something that transforms namelist variable and record names as appropriate for the host model. For CAM-SIMA, this means removing any config_ and mpas_ prefixes, then prepending mpas_. For stand-alone MPAS-A, this means returning the original namelist variable or record name. Then, we would always invoke transform_name, and the implementation of the transform_name function would be the only place where we have #ifdef MPAS_CAM_DYCORE?

I have renamed and reworked the mangle_name function in 7e12d19. Now, it is always called for each MPAS namelist group and option name. If MPAS_CAM_DYCORE is defined, the name mangling will be effective. Otherwise, if MPAS_CAM_DYCORE is not defined, MPAS namelist groups and options will keep their original names as in the registry.

Thanks for the updates! I think having fewer #ifdefs in the code will help readability.

mgduda · 2025-02-19T23:09:54Z

src/tools/registry/gen_inc.c

+
+
+#ifdef MPAS_CAM_DYCORE
+// Perform transformations for namelist group and option names.


We have obviously not done a good job of documenting functions in the past, but I think we're getting better (see, e.g., 3bd1cdd, which modifies the src/registry/utility.c code). Especially if we rethink the usage of the transform_name function, I think it might be nice to include more extensive documentation here.

As above, collectively addressed in 7e12d19.

Thanks! The documentation for mangle_name looks great.

src/tools/registry/gen_inc.c

mgduda

Thanks for the changes -- they look good to me!

Before I merge this PR, I think it would be fine to squash the fixup commits into the first commit of the branch.

…-SIMA When MPAS is used as a dynamical core in a host model (e.g., CAM/CAM-SIMA), the following transformations are performed for each namelist group and option name: 1. Leading "config_" is removed recursively from the name. Case-insensitive. 2. Leading "mpas_" is removed recursively from the name. Case-insensitive. 3. Prepend "mpas_" to the name. By doing so, it is now easier to distinguish MPAS namelist groups and options from host model ones. Additionally, the possibility of name collisions with host model ones is also resolved once and for all. Note that only namelist I/O is affected. Internally, MPAS still refers to its namelist options by their original names due to compatibility reasons. For stand-alone MPAS, its namelist groups and options keep their original names as in the registry.

kuanchihwang · 2025-02-28T21:16:41Z

Thanks! I have squashed the fixup commits into the first one. Additionally, the commit message has been updated to include the clarification that for stand-alone MPAS, its namelist groups and options keep their original names as in the registry. The commit contents remain the same as can be seen by comparison.

mgduda added feature Framework labels Jan 30, 2025

mgduda requested review from mgduda and abishekg7 January 30, 2025 22:12

kuanchihwang force-pushed the staging/dycore-prefix-namelist branch from 183fed2 to 930e9f9 Compare January 31, 2025 18:31

kuanchihwang force-pushed the staging/dycore-prefix-namelist branch from 930e9f9 to b026aa7 Compare February 13, 2025 22:19

abishekg7 approved these changes Feb 18, 2025

View reviewed changes

mgduda requested changes Feb 19, 2025

View reviewed changes

mgduda self-requested a review February 28, 2025 17:26

mgduda approved these changes Feb 28, 2025

View reviewed changes

kuanchihwang force-pushed the staging/dycore-prefix-namelist branch from 7e12d19 to 71f3d22 Compare February 28, 2025 21:09

mgduda merged commit 657794f into MPAS-Dev:develop Feb 28, 2025

kuanchihwang deleted the staging/dycore-prefix-namelist branch February 28, 2025 21:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prefix all namelist group and option names for MPAS dycore #1285

Prefix all namelist group and option names for MPAS dycore #1285

kuanchihwang commented Jan 30, 2025 •

edited

Loading

abishekg7 commented Feb 7, 2025

kuanchihwang commented Feb 7, 2025

abishekg7 commented Feb 7, 2025

kuanchihwang commented Feb 7, 2025 •

edited

Loading

abishekg7 commented Feb 8, 2025

kuanchihwang commented Feb 13, 2025

abishekg7 left a comment

mgduda Feb 19, 2025

kuanchihwang Feb 24, 2025

mgduda Feb 28, 2025

mgduda Feb 19, 2025

kuanchihwang Feb 24, 2025

mgduda Feb 28, 2025

mgduda left a comment

kuanchihwang commented Feb 28, 2025



		#ifdef MPAS_CAM_DYCORE
		// Perform transformations for namelist group and option names.

Prefix all namelist group and option names for MPAS dycore #1285

Prefix all namelist group and option names for MPAS dycore #1285

Conversation

kuanchihwang commented Jan 30, 2025 • edited Loading

abishekg7 commented Feb 7, 2025

kuanchihwang commented Feb 7, 2025

abishekg7 commented Feb 7, 2025

kuanchihwang commented Feb 7, 2025 • edited Loading

abishekg7 commented Feb 8, 2025

kuanchihwang commented Feb 13, 2025

abishekg7 left a comment

Choose a reason for hiding this comment

mgduda Feb 19, 2025

Choose a reason for hiding this comment

kuanchihwang Feb 24, 2025

Choose a reason for hiding this comment

mgduda Feb 28, 2025

Choose a reason for hiding this comment

mgduda Feb 19, 2025

Choose a reason for hiding this comment

kuanchihwang Feb 24, 2025

Choose a reason for hiding this comment

mgduda Feb 28, 2025

Choose a reason for hiding this comment

mgduda left a comment

Choose a reason for hiding this comment

kuanchihwang commented Feb 28, 2025

kuanchihwang commented Jan 30, 2025 •

edited

Loading

kuanchihwang commented Feb 7, 2025 •

edited

Loading