Implement namelist reading and early initialization of MPAS dynamical core #248

kuanchihwang · 2024-02-15T17:40:31Z

This PR implements namelist reading (dyn_readnl) and early initialization of MPAS dynamical core. A new MPAS subdriver module (dyn_mpas_subdriver) is introduced, which will oversee the life cycle (i.e., initialization, running, and finalization) of MPAS as a dynamical core within CAM-SIMA.

Credits to Dylan Dickerson (@gdicker1) for authoring namelist_definition_mpas_dycore.xml.

Implementation notes:

OOP design for the MPAS subdriver module.
- A "class" (i.e., derived type) of MPAS dynamical core is publicly accessible.
- Important data structures like states of MPAS dynamical core are encapsulated inside this derived type to prevent misuse.
- Type-bound procedures provide well-defined APIs for CAM-SIMA to interact with MPAS dynamical core.
Another difference is the approach taken to read namelist. Currently in CAM, namelist is read manually by CAM itself [1] on behalf of MPAS. This seems like a maintenance burden when the contents of upstream MPAS namelist evolve over time. Therefore, in this implementation, we call upstream MPAS functionality [2] to handle its own namelist for us.

Test steps:

"<path-to>/CAM-SIMA/cime/scripts/create_newcase" --case "<case-name>" --compset FKESSLER --project "<project-id>" --res mpasa480_mpasa480 --run-unsupported
cd "<case-name>"
./case.setup
# Edit `env_build.xml`:
# * Change `CAM_LINKED_LIBS` to only `-lmpas`.
# * Change `DEBUG` to `TRUE`.
./case.build
./case.submit

Observe the following log entries in atm.log.<job-id>.<date>-<time>:

dyn_mpas_debug_print (0): dyn_mpas_subdriver::dyn_mpas_init_phase1 entered
dyn_mpas_debug_print (0): Allocating core
dyn_mpas_debug_print (0): Allocating domain
dyn_mpas_debug_print (0): Calling mpas_framework_init_phase1
dyn_mpas_debug_print (0): Setting up core
dyn_mpas_debug_print (0): Setting up domain
dyn_mpas_debug_print (0): Setting up log
dyn_mpas_debug_print (0): dyn_mpas_subdriver::dyn_mpas_init_phase1 completed
dyn_mpas_debug_print (0): dyn_mpas_subdriver::dyn_mpas_read_namelist entered
dyn_mpas_debug_print (0): Reading namelist at atm_in
dyn_mpas_debug_print (0): Overriding designated namelist variables
dyn_mpas_debug_print (0): config_calendar_type = gregorian_noleap
dyn_mpas_debug_print (0): config_start_time = 1-1-1_0:0:0
dyn_mpas_debug_print (0): config_stop_time = 1-1-6_0:0:0
dyn_mpas_debug_print (0): config_run_duration = 5_0:0:0
dyn_mpas_debug_print (0): config_do_restart = F
dyn_mpas_debug_print (0): dyn_mpas_subdriver::dyn_mpas_read_namelist completed
dyn_mpas_debug_print (0): dyn_mpas_subdriver::dyn_mpas_init_phase2 entered
dyn_mpas_debug_print (0): Checking PIO system descriptor
dyn_mpas_debug_print (0): Calling mpas_framework_init_phase2
dyn_mpas_debug_print (0): dyn_mpas_subdriver::dyn_mpas_init_phase2 completed

The history of time manager has diverged a lot between CAM and CAM-SIMA. Manually backport `get_stop_date` and `get_run_duration` subroutines to time manager for MPAS.

This module manages the life cycle (i.e., initialization, running, and finalization) of MPAS as a dynamical core within CAM-SIMA.

`dyn_readnl` calls appropriate MPAS subdriver APIs to: 1. Read MPAS namelist from supplied path. 2. Perform early initialization of MPAS dynamical core.

MPAS subdriver is built and included in `libmpas.a` static library.

Also fix a bug in `_copy2_as_needed`. Previously, if `dst` is a directory and `os.path.join(dst, os.path.basename(src))` is a file, the file will be copied again unconditionally. In this case, checks should be performed first.

mgduda · 2024-02-21T18:28:40Z

I believe the namelist_definition_mpas_dycore.xml file was (mostly?) automatically generated from the stand-alone MPAS-A Registry.xml file with a Python script.

@gdicker1 @kuanchihwang Would it be worth committing the script used to generate namelist_definition_mpas_dycore.xml somewhere in CAM-SIMA so that it's under version control? I think having this script around might make it easier to update CAM-SIMA namelist options as the MPAS-A dycore changes in future.

gdicker1 · 2024-02-21T18:34:10Z

It's ugly and quick, so I wonder if the script is "ready" for CAM-SIMA. Give me a few to get it into somewhere on GitHub so I can link it for others to judge.

mgduda · 2024-02-21T18:42:34Z

Some of the additions in this PR are generic (in that they are not MPAS-A dycore-specific); for example, the new routines in src/utils/time_manager.F90.

@nusbaume I'm not sure if there's anything covering this in the CAM development workflow or coding standards pages, but is it preferable to make separate PRs in cases like this?

src/dynamics/mpas/driver/dyn_mpas_subdriver.F90

gdicker1 · 2024-02-21T18:53:32Z

It's ugly and quick, so I wonder if the script is "ready" for CAM-SIMA. Give me a few to get it into somewhere on GitHub so I can link it for others to judge.

Here it is! Turns out I had it in a repo already: https://github.com/gdicker1/TranslateXML_MPAStoCAMSIMA

src/dynamics/mpas/dyn_comp.F90

src/dynamics/mpas/driver/dyn_mpas_subdriver.F90

mgduda · 2024-02-21T21:19:57Z

It's ugly and quick, so I wonder if the script is "ready" for CAM-SIMA. Give me a few to get it into somewhere on GitHub so I can link it for others to judge.

Here it is! Turns out I had it in a repo already: https://github.com/gdicker1/TranslateXML_MPAStoCAMSIMA

That seems more than clean enough to me. As a question to all, would it make sense to commit that script to, e.g., somewhere in the CAM-SIMA repo like src/dynamics/mpas/scripts/, or to keep it in a separate repo and reference that in a comment at the top of the namelist_definition_mpas_dycore.xml file? Essentially, as we need to update the MPAS-A dycore namelist options in CAM-SIMA, it would be nice to not have to go on a quest to find the Python script each time.

If `NDEBUG` macro is defined (i.e., release builds), debug prints are disabled.

Introduce `printer` optional argument to `dyn_mpas_debug_print` subroutine. If `printer` is not supplied, the MPI master rank will print. Otherwise, the designated MPI rank will print instead.

... In accordance with the spirit of OOP.

nusbaume · 2024-02-26T21:39:54Z

I personally think it's fine to bring in code that is not strictly MPAS-specific in an MPAS PR, just as long as the PR itself is not too large (this one is a good size, for example).

Also I'd be happy to have the registry translation script live inside CAM-SIMA, especially since it seems pretty SIMA-specific (i.e. I assume it won't be used elsewhere?). We discussed it in a meeting today and decided that having it sit somewhere under src/dynamics/mpas makes sense to us.

Finally, I just wanted to let you know that I have finally started my review of this PR and should get any comments I have posted here soon. Thanks!

mgduda · 2024-02-26T22:44:48Z

As a broader comment/question, should we consider "namespacing" the MPAS-A dycore namelist options somehow? For example, right now in this PR we have this namelist group:

&damping
    config_mpas_cam_coef = 0.0
    config_number_cam_damping_levels = 4
    config_number_rayleigh_damp_u_levels = 6
    config_rayleigh_damp_u = .false.
    config_rayleigh_damp_u_timescale_days = 5.0
    config_xnutr = 0.2
    config_zd = 22000.0
/

One of the variables in this group does have the "mpas" substring, but for others, there's nothing that indicates that these apply to the MPAS-A dycore. I believe @gdicker1's script substituted config_ for mpas_ (and removed duplicate mpas_ substrings), and perhaps retaining something of that could be useful.

I completely appreciate the desire to use MPAS-A's native namelist reading routines; but that does come at the cost of namelist variable and namelist group names in CAM-SIMA that aren't obviously tied to the MPAS-A dycore.

At a minimum, I think it would be good to:

Explore the possibility of at least prefixing the namelist groups with mpas_, e.g., &mpas_damping
Consider how great the burden of maintaining separate namelist reading code in CAM-SIMA might be given that we already need to maintain the namelist_definition_mpas_dycore.xml file.

src/dynamics/mpas/driver/dyn_mpas_subdriver.F90

Avoid double negation in expression.

nusbaume

Nice work! I have a couple of potentially significant requests, but everything else will hopefully be minor. Of course please let me know if you have any questions or concerns with any of my comments. Thanks!

src/dynamics/mpas/driver/dyn_mpas_subdriver.F90

src/dynamics/mpas/dyn_comp.F90

src/dynamics/mpas/namelist_definition_mpas_dycore.xml

Note that the operation is idempotent due to the optional capturing group `(mpas_)?` in regexp.

kuanchihwang · 2024-02-28T20:42:07Z

As a broader comment/question, should we consider "namespacing" the MPAS-A dycore namelist options somehow?
...

@mgduda @nusbaume

After some investigations, I think we can have the best of both worlds. Hopefully 8669b0e can address your comments.

All MPAS namelist groups are now prefixed by mpas_, and we still get to use the native MPAS functionality to read its namelist. For example, the damping namelist group that you mentioned now looks like this in atm_in:

&mpas_damping
    config_mpas_cam_coef = 0.0
    config_number_cam_damping_levels = 4
    config_number_rayleigh_damp_u_levels = 6
    config_rayleigh_damp_u = .false.
    config_rayleigh_damp_u_timescale_days = 5.0
    config_xnutr = 0.2
    config_zd = 22000.0
/

So it will not to be confused with others.

kuanchihwang · 2024-02-28T20:49:08Z

About bringing the "MPAS registry -> CAM-SIMA namelist definition" conversion script, I think it should be in another PR.

nusbaume

This PR looks good to me now, thanks @kuanchihwang! I had one last request about adding a comment header to namelist_definition_mpas_dycore.xml, and a general comment about the possible future risk of a namelist collision (although I think for now the chances are low).

Also it's totally fine with me if you want to save the addition of the registry translation script for another PR.

src/dynamics/mpas/namelist_definition_mpas_dycore.xml

nusbaume · 2024-02-29T20:04:18Z

src/dynamics/mpas/namelist_definition_mpas_dycore.xml

@@ -4,7 +4,7 @@
  <entry id="config_time_integration">
    <type>char*256</type>
    <category>mpas</category>
-    <group>nhyd_model</group>
+    <group>mpas_nhyd_model</group>


Labeling the group name should definitely help! However, during the actual namelist generation step there is still a chance of a name collision as the current workflow is for the user to only include the namelist variable name in the user_nl_cam file. So long term it might be good to find some way to add an mpas string to each MPAS-A namelist variable as well.

That all being said, I think the general odds of a collision are low as the namelist variable strings would have to match exactly, so I don't think this concern should hold up the PR. Plus if we do run into a name collision then we can always fix that particular name then.

gdicker1 · 2024-02-29T20:44:06Z

About bringing the "MPAS registry -> CAM-SIMA namelist definition" conversion script, I think it should be in another PR.

I have created an Issue to track this and follow-up later: #252

gdicker1 and others added 8 commits February 14, 2024 15:12

Add XML namelist definition file for MPAS dynamical core

d985991

Wire up XML namelist definition file in CAM config

3b42c4f

Manually backport necessary subroutines to time manager for MPAS

ef48506

The history of time manager has diverged a lot between CAM and CAM-SIMA. Manually backport `get_stop_date` and `get_run_duration` subroutines to time manager for MPAS.

Start implementing MPAS subdriver

a217fdf

This module manages the life cycle (i.e., initialization, running, and finalization) of MPAS as a dynamical core within CAM-SIMA.

Implement dyn_readnl

3b27365

`dyn_readnl` calls appropriate MPAS subdriver APIs to: 1. Read MPAS namelist from supplied path. 2. Perform early initialization of MPAS dynamical core.

Build MPAS subdriver with upstream

f9c6a1b

MPAS subdriver is built and included in `libmpas.a` static library.

Build MPAS subdriver with upstream

c348ea5

Also fix a bug in `_copy2_as_needed`. Previously, if `dst` is a directory and `os.path.join(dst, os.path.basename(src))` is a file, the file will be copied again unconditionally. In this case, checks should be performed first.

Fix wrong prefix in XML namelist definition file

9494b82

kuanchihwang requested review from mgduda and nusbaume February 15, 2024 17:40

Adjust indentation level

363d782

mgduda reviewed Feb 21, 2024

View reviewed changes

src/dynamics/mpas/driver/dyn_mpas_subdriver.F90 Outdated Show resolved Hide resolved

mgduda reviewed Feb 21, 2024

View reviewed changes

src/dynamics/mpas/dyn_comp.F90 Outdated Show resolved Hide resolved

mgduda reviewed Feb 21, 2024

View reviewed changes

src/dynamics/mpas/driver/dyn_mpas_subdriver.F90 Outdated Show resolved Hide resolved

mgduda reviewed Feb 21, 2024

View reviewed changes

src/dynamics/mpas/driver/dyn_mpas_subdriver.F90 Outdated Show resolved Hide resolved

kuanchihwang added 6 commits February 21, 2024 14:34

Reword a comment

5b90e51

Whitespace around construct

7b4633b

Adjust indentation level

5ef859f

Disable debug prints in release builds

ce9bc5f

If `NDEBUG` macro is defined (i.e., release builds), debug prints are disabled.

Default to MPI master rank for debug prints

3eaeeb8

Introduce `printer` optional argument to `dyn_mpas_debug_print` subroutine. If `printer` is not supplied, the MPI master rank will print. Otherwise, the designated MPI rank will print instead.

Move instantiation of MPAS dynamical core to dyn_comp module

c4fda96

... In accordance with the spirit of OOP.

mgduda reviewed Feb 26, 2024

View reviewed changes

src/dynamics/mpas/driver/dyn_mpas_subdriver.F90 Outdated Show resolved Hide resolved

Use DEBUG macro instead

361d187

Avoid double negation in expression.

nusbaume requested changes Feb 27, 2024

View reviewed changes

kuanchihwang added 7 commits February 27, 2024 14:17

Prefix mpas_ to MPAS namelist groups to avoid naming collisions

8669b0e

Note that the operation is idempotent due to the optional capturing group `(mpas_)?` in regexp.

Prefix mpas_ to MPAS namelist groups to avoid naming collisions

c58059b

Change wording

0e62d59

Add more comments for stringify pure function

4a34799

Add stringify pure function to string_utils module

c8e09ca

Make debug output run-time configurable

176e00b

Add more comments for dyn_readnl subroutine

6e5d6ab

kuanchihwang requested review from mgduda and nusbaume February 28, 2024 20:54

nusbaume approved these changes Feb 29, 2024

View reviewed changes

gdicker1 mentioned this pull request Feb 29, 2024

Add script to create namelist_definition_mpas_dycore.xml from MPAS-A Registry.xml #252

Open

Add more comments for XML namelist definition file

4c0f612

mgduda approved these changes Mar 5, 2024

View reviewed changes

kuanchihwang merged commit 3f54ce1 into ESCOMP:development Mar 5, 2024
6 checks passed

kuanchihwang deleted the staging/implement-dyn-readnl branch March 5, 2024 23:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement namelist reading and early initialization of MPAS dynamical core #248

Implement namelist reading and early initialization of MPAS dynamical core #248

kuanchihwang commented Feb 15, 2024 •

edited

Loading

mgduda commented Feb 21, 2024

gdicker1 commented Feb 21, 2024 •

edited

Loading

mgduda commented Feb 21, 2024

gdicker1 commented Feb 21, 2024 •

edited

Loading

mgduda commented Feb 21, 2024

nusbaume commented Feb 26, 2024

mgduda commented Feb 26, 2024

nusbaume left a comment

kuanchihwang commented Feb 28, 2024

kuanchihwang commented Feb 28, 2024

nusbaume left a comment

nusbaume Feb 29, 2024

gdicker1 commented Feb 29, 2024

Implement namelist reading and early initialization of MPAS dynamical core #248

Implement namelist reading and early initialization of MPAS dynamical core #248

Conversation

kuanchihwang commented Feb 15, 2024 • edited Loading

Implementation notes:

Test steps:

mgduda commented Feb 21, 2024

gdicker1 commented Feb 21, 2024 • edited Loading

mgduda commented Feb 21, 2024

gdicker1 commented Feb 21, 2024 • edited Loading

mgduda commented Feb 21, 2024

nusbaume commented Feb 26, 2024

mgduda commented Feb 26, 2024

nusbaume left a comment

Choose a reason for hiding this comment

kuanchihwang commented Feb 28, 2024

kuanchihwang commented Feb 28, 2024

nusbaume left a comment

Choose a reason for hiding this comment

nusbaume Feb 29, 2024

Choose a reason for hiding this comment

gdicker1 commented Feb 29, 2024

kuanchihwang commented Feb 15, 2024 •

edited

Loading

gdicker1 commented Feb 21, 2024 •

edited

Loading

gdicker1 commented Feb 21, 2024 •

edited

Loading