Implement finalization and various enhancements for MPAS dynamical core #327

kuanchihwang · 2024-12-03T00:23:22Z

Tag name (required for release branches):

None

Originator(s):

kuanchihwang

Descriptions (include the issue title, and the keyword ['closes', 'fixes', 'resolves'] followed by the issue number):

This PR implements finalization for MPAS dynamical core.

This PR also implements various enhancements for MPAS dynamical core:

Comply with CAM(-SIMA) coding standards by moving use statements to the smallest scope possible.
Support MPAS dynamical core in single precision mode. Running in single precision mode improves performance and reduces memory usage at the cost of accuracy.
Reorganize directory structure for better clarity. Files that are not source code are now placed in the assets directory.
Prefix all MPAS namelist group and option names with mpas_. They are now easily distinguishable from CAM-SIMA ones. The possibility of name collisions with CAM-SIMA ones is also resolved once and for all.
Add Python script for generating namelist definition. Closes Add script to create namelist_definition_mpas_dycore.xml from MPAS-A Registry.xml #252.
Update namelist definition so that MPAS dynamical core should now be able to run analytic cases out of the box.
Various code improvements.

Describe any changes made to the build system:

Due to the slight reorganization of directory structure, cime_config/buildlib is updated with the new paths.

Describe any changes made to the namelist:

The namelist options for running analytic cases with MPAS dynamical core are updated to match CAM.

List any changes to the defaults for the input datasets (e.g., boundary datasets):

The input files for running analytic cases with MPAS dynamical core are updated to match CAM. The supported resolutions are:

Horizontal resolutions of mpasa480, mpasa120, mpasa60, and mpasa30 with 32 vertical layers.
Horizontal resolutions of mpasa480, mpasa120, and mpasa60 with 58 vertical layers.
Horizontal resolutions of mpasa480, mpasa120, and mpasa60 with 93 vertical layers.

List all files eliminated and why:

None

List all files added and what they do:

A src/dynamics/mpas/assets/0001-Prefix-all-MPAS-namelist-group-and-option-names.patch
- Patch that prefixes all MPAS namelist group and option names
A src/dynamics/mpas/assets/0002-Disable-physics-for-MPAS-dycore-only-build.patch
- Patch that disables physics for MPAS dycore-only build
A src/dynamics/mpas/assets/0003-Declare-constants-at-the-native-precision-of-MPAS.patch
- Patch that declares constants at the native precision of MPAS
A src/dynamics/mpas/assets/generate_namelist_definition.py
- Add Python script for generating namelist definition

List all existing files that have been modified, and describe the changes:

M cime_config/buildlib
- Reorganize directory structure for better clarity
M cime_config/namelist_definition_cam.xml
- Update namelist definition for CAM-SIMA
M src/dynamics/mpas/driver/dyn_mpas_subdriver.F90
- Add support for MPAS output stream
- Relax restrictions on mixed precision
- Use existing functionality to access MPAS configuration
- Implement finalization for MPAS dynamical core
- Move use statements to the smallest scope possible
- Support MPAS dynamical core in single precision mode
M src/dynamics/mpas/dyn_comp.F90
- Implement dyn_final
- Move use statements to the smallest scope possible
- Support MPAS dynamical core in single precision mode
- Add protected attribute to grid/mesh variables
- Change some wording to plural
- Include argument keyword for better clarity
M src/dynamics/mpas/dyn_coupling.F90
- Move use statements to the smallest scope possible
- Support MPAS dynamical core in single precision mode
- Change some wording to plural
- Include argument keyword for better clarity
M src/dynamics/mpas/dyn_grid.F90
- Move use statements to the smallest scope possible
- Support MPAS dynamical core in single precision mode
- Nullify pointers that are no longer needed
- Add protected attribute to grid/mesh variables
M src/dynamics/mpas/namelist_definition_mpas_dycore.xml
- Update namelist definition for MPAS dynamical core
M src/dynamics/mpas/stepon.F90
- Wire up dyn_final
- Move use statements to the smallest scope possible
R src/dynamics/mpas/assets/Makefile.in.CESM
- Reorganize directory structure for better clarity
- Remove hacky sed commands from Makefile
- Apply patches before building
R src/dynamics/mpas/assets/Makefile
- Reorganize directory structure for better clarity

Regression tests:

New baseline for MPAS dynamical core.

  SMS_Ln9.mpasa480_mpasa480.FKESSLER.derecho_gnu.cam-outfrq_kessler_mpas_derecho (Overall: DIFF) details:
    PASS SMS_Ln9.mpasa480_mpasa480.FKESSLER.derecho_gnu.cam-outfrq_kessler_mpas_derecho CREATE_NEWCASE
    PASS SMS_Ln9.mpasa480_mpasa480.FKESSLER.derecho_gnu.cam-outfrq_kessler_mpas_derecho XML
    PASS SMS_Ln9.mpasa480_mpasa480.FKESSLER.derecho_gnu.cam-outfrq_kessler_mpas_derecho SETUP
    PASS SMS_Ln9.mpasa480_mpasa480.FKESSLER.derecho_gnu.cam-outfrq_kessler_mpas_derecho SHAREDLIB_BUILD time=10
    FAIL SMS_Ln9.mpasa480_mpasa480.FKESSLER.derecho_gnu.cam-outfrq_kessler_mpas_derecho NLCOMP
    PASS SMS_Ln9.mpasa480_mpasa480.FKESSLER.derecho_gnu.cam-outfrq_kessler_mpas_derecho MODEL_BUILD time=137
    PASS SMS_Ln9.mpasa480_mpasa480.FKESSLER.derecho_gnu.cam-outfrq_kessler_mpas_derecho SUBMIT
    PASS SMS_Ln9.mpasa480_mpasa480.FKESSLER.derecho_gnu.cam-outfrq_kessler_mpas_derecho RUN time=17
    PASS SMS_Ln9.mpasa480_mpasa480.FKESSLER.derecho_gnu.cam-outfrq_kessler_mpas_derecho GENERATE
    FAIL SMS_Ln9.mpasa480_mpasa480.FKESSLER.derecho_gnu.cam-outfrq_kessler_mpas_derecho BASELINE
    PASS SMS_Ln9.mpasa480_mpasa480.FKESSLER.derecho_gnu.cam-outfrq_kessler_mpas_derecho MEMLEAK
    PASS SMS_Ln9.mpasa480_mpasa480.FKESSLER.derecho_gnu.cam-outfrq_kessler_mpas_derecho SHORT_TERM_ARCHIVER

No changes to other existing tests.

kuanchihwang · 2024-12-03T00:30:14Z

Additional comments:

Note to reviewers: It might be easier to review the PR commit by commit. The commit messages provide the context and rationale behind each set of changes.

You should be able to run analytic cases currently supported by CAM-SIMA (e.g., FHS94, FKESSLER) with MPAS dynamical core. No tinkering with user_nl_cam or XML files is required.

You are also encouraged to test MPAS dynamical core in single precision mode. From the following benchmarks, MPAS dynamical core in single precision mode is ~40% faster and uses ~25% less memory than in double precision mode. A reference CAM run with the same configuration is also provided for completeness.

* Adjust wording and keep code comments up-to-date * Concentrate all calls to `mark_as_initialized` in one place * Fix up code style inconsistencies

Also fix variable rank information.

Relax restrictions on mixed precision between MPAS dynamical core and its input data.

`get_variable_pointer` also works for accessing MPAS configuration. It has built-in error checking so no need to do it manually.

Comply with CAM(-SIMA) coding standards. However, kind and length parameters are exempt from this rule. They are better at module level.

It is now possible to run CAM-SIMA with MPAS dynamical core in single precision mode. It can be enabled by defining the `SINGLE_PRECISION` macro in `CPPFLAGS`.

Protect these variables from being modified externally.

Manipulating source code by running `sed` commands during building is neither elegant nor obvious. Downstream modifications like these are better achieved and communicated through patches. This is also the standard practice in open source software.

These patches are applied before building.

This Python script generates the XML namelist definition file of MPAS dynamical core from MPAS registry. Optionally, if an XML schema is provided, the generated file can also be validated for correctness.

From now on, it can be generated automatically from MPAS registry.

Add MPAS initial files for running analytic cases.

nusbaume

Looks great @kuanchihwang! I just have a few questions and some very minor requests, mostly to move some remaining module-level use statements to their respective subroutines. Of course if any of my requests are problematic just let me know. Thanks!

nusbaume · 2024-12-26T21:19:08Z

src/dynamics/mpas/assets/Makefile.in.CESM

+# Uncomment below for single precision mode support. There is currently no corresponding option in CIME to enable this.
+# export CPPFLAGS       += -DSINGLE_PRECISION


Not worth trying to resolve in this PR, but I believe we could make the use of single precision an option in the CAM_CONFIG_OPTS variable in the CIME env_build.xml file, where then the CAM-SIMA configuration system can set an environment variable that this Makefile then queries to set the relevant CPP Flag (or alternatively we could just add it directly to the full CAM-SIMA Makefile). This would provide a way for the average user to change the precision used in their own simulation without having to modify any source files.

Does this sound reasonable to you? If so then we should make a Github issue to remind us to implement it in a later PR.

nusbaume · 2024-12-27T01:57:11Z

src/dynamics/mpas/assets/Makefile.in.CESM

+	@for file in *.patch; do \
+		if git apply --check -p2 "$${file}"; then \
+			echo "Applying $${file}"; \
+			git apply -p2 "$${file}"; \
+		fi; \


Do you have any sense what the medium or long-term maintainability of this method will be? For example, will we need to create new patch files every time we update the MPAS submodule in CAM-SIMA, or are the targeted files rarely ever modified?

I don't think it matters for this PR, but if you think that it could require an abnormal amount of work to maintain then it might be worth opening a Github issue so we can remember to try and figure out alternative methods in the future.

nusbaume · 2024-12-27T21:19:52Z

src/dynamics/mpas/assets/generate_namelist_definition.py

+        default='Namelist.xml',
+        type=str,
+        required=False,
+        help='XML namelist definition file.',


I might specify that this is a CAM-SIMA namelist definition file here:

Suggested change

help='XML namelist definition file.',

help='XML CAM-SIMA namelist definition file.',

nusbaume · 2024-12-27T21:38:00Z

src/dynamics/mpas/assets/generate_namelist_definition.py

+        INDENT_PER_LEVEL * 2 + 'MPAS dycore' + '\n' +
+        '\n' +
+        INDENT_PER_LEVEL * 2 + 'Note to developers/maintainers:' + '\n' +
+        INDENT_PER_LEVEL * 2 + 'This file is auto-generated from MPAS registry. Do not edit directly.' + '\n' +


Minor grammar nit-pick:

Suggested change

INDENT_PER_LEVEL * 2 + 'This file is auto-generated from MPAS registry. Do not edit directly.' + '\n' +

INDENT_PER_LEVEL * 2 + 'This file is auto-generated from the MPAS registry. Do not edit directly.' + '\n' +

nusbaume · 2024-12-28T04:33:19Z

src/dynamics/mpas/stepon.F90

-    use time_manager, only: get_step_size
-
-    ! Modules from CCPP.
+    ! Module(s) from CCPP.
    use ccpp_kinds, only: kind_phys


I believe this use statement can be passed down to the subroutine scope as well (i.e. there don't appear to be any module-level variables that need it).

nusbaume · 2024-12-28T16:58:08Z

src/dynamics/mpas/dyn_coupling.F90

+    ! Module(s) from CESM Share.
+    use shr_kind_mod, only: kind_r8 => shr_kind_r8, &
+                            len_cx => shr_kind_cx
+    ! Module(s) from MPAS.
+    use dyn_mpas_subdriver, only: kind_dyn_mpas => mpas_dynamical_core_real_kind


I think all of these can be moved to the subroutine scope as well.

nusbaume · 2024-12-28T20:46:14Z

src/dynamics/mpas/dyn_comp.F90

+    ! Module(s) from CESM Share.
+    use shr_kind_mod, only: kind_r8 => shr_kind_r8, &
+                            len_cs => shr_kind_cs


I believe this use shr_kind_mod statement can be moved to the subroutine scope.

nusbaume · 2024-12-29T05:09:57Z

src/dynamics/mpas/dyn_comp.F90

+        call dyn_debug_print('ncells_global    = ' // stringify([ncells_global]))
+        call dyn_debug_print('nedges_global    = ' // stringify([nedges_global]))
+        call dyn_debug_print('nvertices_global = ' // stringify([nvertices_global]))
+        call dyn_debug_print('nvertlevels      = ' // stringify([nvertlevels]))
+        call dyn_debug_print('ncells_max       = ' // stringify([ncells_max]))
+        call dyn_debug_print('nedges_max       = ' // stringify([nedges_max]))
+        call dyn_debug_print('sphere_radius    = ' // stringify([sphere_radius]))


Should we provide debuging print outs for the local mesh dimensions as well?

nusbaume · 2024-12-29T05:53:59Z

src/dynamics/mpas/dyn_comp.F90

+        ! Quick hack for dumping variables from MPAS dynamical core.
+        ! Remove it once history and restart are wired up in CAM-SIMA.
+        call dyn_variable_dump()


Just an FYI that CAM-SIMA does have history infrastructure now for any diagnostic variable that can be output instantaneously (i.e. doesn't need to be accumulated over multiple timesteps, like an average). You can find a description and user guide for this new history infrastructure in the official CAM-SIMA developer documentation here:

https://escomp.github.io/CAM-SIMA-docs/design/history/#capturing-history-output

We sadly don't have restart functionality enabled yet, but hopefully this can provide at least some guidance for how to start replacing this "hack" in the future.

nusbaume · 2024-12-29T22:42:50Z

src/dynamics/mpas/driver/dyn_mpas_subdriver.F90

-    use mpas_io_streams, only: mpas_createstream, mpas_closestream, mpas_streamaddfield, &
-                               mpas_readstream, mpas_writestream, mpas_writestreamatt
+    ! Module(s) from MPAS.
+    use mpas_derived_types, only: core_type, domain_type
    use mpas_kind_types, only: rkind, r4kind, r8kind, strkind


I believe r4kind and r8kind can be moved to the subroutine scope (as they appear to only be used in dyn_mpas_check_variable_status).

kuanchihwang force-pushed the staging/implement-dyn-final branch 3 times, most recently from cd464ea to 90fc72a Compare December 10, 2024 19:36

kuanchihwang added 20 commits December 16, 2024 16:13

Tidy up MPAS-related changes introduced in PR ESCOMP#316

ff30858

* Adjust wording and keep code comments up-to-date * Concentrate all calls to `mark_as_initialized` in one place * Fix up code style inconsistencies

Add support for MPAS output stream

14f7019

Also fix variable rank information.

Relax restrictions on mixed precision

b7410ce

Relax restrictions on mixed precision between MPAS dynamical core and its input data.

Use existing functionality to access MPAS configuration

0c0d94d

`get_variable_pointer` also works for accessing MPAS configuration. It has built-in error checking so no need to do it manually.

Implement finalization for MPAS dynamical core

2dd1e95

Implement dyn_final

f7b459b

Wire up dyn_final

2e7ac8f

Move use statements to the smallest scope possible

cbb43cf

Comply with CAM(-SIMA) coding standards. However, kind and length parameters are exempt from this rule. They are better at module level.

Support MPAS dynamical core in single precision mode

5a4d3bc

It is now possible to run CAM-SIMA with MPAS dynamical core in single precision mode. It can be enabled by defining the `SINGLE_PRECISION` macro in `CPPFLAGS`.

Nullify pointers that are no longer needed

4491172

Add protected attribute to grid/mesh variables

b32fe78

Protect these variables from being modified externally.

Change some wording to plural

bc45c67

Include argument keyword for better clarity

be1449f

Reorganize directory structure for better clarity

efd27e8

Convert CAM-SIMA-specific modifications to MPAS into patches

40b1ef9

These patches are applied before building.

Apply patches before building

cdc0b8e

Add Python script for generating namelist definition

a05b30f

This Python script generates the XML namelist definition file of MPAS dynamical core from MPAS registry. Optionally, if an XML schema is provided, the generated file can also be validated for correctness.

Update namelist definition for MPAS dynamical core

6c43d37

From now on, it can be generated automatically from MPAS registry.

Update namelist definition for CAM-SIMA

e36fcdb

Add MPAS initial files for running analytic cases.

kuanchihwang force-pushed the staging/implement-dyn-final branch from 90fc72a to e36fcdb Compare December 16, 2024 23:42

kuanchihwang marked this pull request as ready for review December 17, 2024 01:06

kuanchihwang requested review from mgduda and nusbaume December 17, 2024 01:06

nusbaume requested changes Dec 30, 2024

View reviewed changes

nusbaume mentioned this pull request Dec 30, 2024

Need documentation about CAM-SIMA directory structure #251

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement finalization and various enhancements for MPAS dynamical core #327

Implement finalization and various enhancements for MPAS dynamical core #327

kuanchihwang commented Dec 3, 2024 •

edited

Loading

kuanchihwang commented Dec 3, 2024 •

edited

Loading

nusbaume left a comment

nusbaume Dec 26, 2024

nusbaume Dec 27, 2024

nusbaume Dec 27, 2024

nusbaume Dec 27, 2024

nusbaume Dec 28, 2024

nusbaume Dec 28, 2024

nusbaume Dec 28, 2024

nusbaume Dec 29, 2024

nusbaume Dec 29, 2024

nusbaume Dec 29, 2024

		# Uncomment below for single precision mode support. There is currently no corresponding option in CIME to enable this.
		# export CPPFLAGS += -DSINGLE_PRECISION

	help='XML namelist definition file.',
	help='XML CAM-SIMA namelist definition file.',

	INDENT_PER_LEVEL * 2 + 'This file is auto-generated from MPAS registry. Do not edit directly.' + '\n' +
	INDENT_PER_LEVEL * 2 + 'This file is auto-generated from the MPAS registry. Do not edit directly.' + '\n' +

Implement finalization and various enhancements for MPAS dynamical core #327

Are you sure you want to change the base?

Implement finalization and various enhancements for MPAS dynamical core #327

Conversation

kuanchihwang commented Dec 3, 2024 • edited Loading

Tag name (required for release branches):

Originator(s):

Descriptions (include the issue title, and the keyword ['closes', 'fixes', 'resolves'] followed by the issue number):

Describe any changes made to the build system:

Describe any changes made to the namelist:

List any changes to the defaults for the input datasets (e.g., boundary datasets):

List all files eliminated and why:

List all files added and what they do:

List all existing files that have been modified, and describe the changes:

Regression tests:

kuanchihwang commented Dec 3, 2024 • edited Loading

Additional comments:

nusbaume left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kuanchihwang commented Dec 3, 2024 •

edited

Loading

kuanchihwang commented Dec 3, 2024 •

edited

Loading