search

ESCOMP / CAM-SIMA

Community Atmosphere Model - System for Integrated Modeling of the Atmosphere
3 stars 11 forks source link

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

Closed kuanchihwang closed 5 months ago

kuanchihwang commented 5 months ago

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:

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
mgduda commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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?

gdicker1 commented 5 months ago

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

mgduda commented 5 months ago

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.

nusbaume commented 5 months ago

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 commented 5 months ago

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:

  1. Explore the possibility of at least prefixing the namelist groups with mpas_, e.g., &mpas_damping
  2. 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.
kuanchihwang commented 5 months ago

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 https://github.com/ESCOMP/CAM-SIMA/pull/248/commits/8669b0e7c212260c6c72a0e7ccc66085160ca5c2 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 commented 5 months ago

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

gdicker1 commented 5 months ago

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