Refactor MPAS dynamical core in preparation for unit testing - #407
Merged
kuanchihwang merged 18 commits intoAug 13, 2025
Merged
Conversation
The dyn_{comp,coupling,grid} MODULES will only contain public interfaces
and variables. This is analogous to the header files (*.h) in C.
The dyn_{comp,coupling,grid}_impl SUBMODULES will contain the actual
implementations for the respective modules. This is analogous to the
source files (*.c) in C.
Step 1/2: Convert the dyn_{comp,coupling,grid} MODULES into
the dyn_{comp,coupling,grid}_impl SUBMODULES.
The dyn_{comp,coupling,grid} MODULES will only contain public interfaces
and variables. This is analogous to the header files (*.h) in C.
The dyn_{comp,coupling,grid}_impl SUBMODULES will contain the actual
implementations for the respective modules. This is analogous to the
source files (*.c) in C.
Step 2/2: Restore the dyn_{comp,coupling,grid} MODULES that only contain
public interfaces and variables.
Move subroutines as well as related variables to the modules that best reflect their functional context. For example, * `dyn_exchange_constituent_states` was moved to `dyn_coupling`. * `dyn_inquire_mesh_dimensions` was moved to `dyn_grid`.
Step 1/2: Factor out easily compilable and testable procedures on CAM-SIMA side into the separate module, `dyn_procedure`.
Step 2/2: Factor out easily compilable and testable procedures on MPAS side into the separate module, `dyn_mpas_procedure`.
Put thermodynamics-related initialization code into separate subroutines so that it does not bloat and pollute the scope of dyn_init.
kuanchihwang
force-pushed
the
staging/mpas-dycore-refactor
branch
from
July 21, 2025 18:23
0656bcc to
ed50c4d
Compare
Collaborator
Author
|
To reviewers, the lines of change might look terrifying, but all of them are just moving things around (i.e., conversion to Fortran submodules and splitting out procedures for unit testing). Absolutely no functionalities are being added or changed. There are no answer changes as confirmed by the passing regression tests. I have also structured the commits in a way that should be easier to follow one by one in order. |
kuanchihwang
marked this pull request as ready for review
July 21, 2025 18:34
Consolidate use statements into one.
nusbaume
added a commit
that referenced
this pull request
Aug 6, 2025
Tag name (required for release branches): Originator(s): nusbaume Description (include the issue title, and the keyword ['closes', 'fixes', 'resolves'] followed by the issue number): Updated the CAM-SIMA external submodules to match cam6_4_106, along with a newer CIME tag to enable dependency checking of Fortran submodules, which is needed for CAM-SIMA PR #407 Also removed the "mct" submodule, as that was no longer being used by CAM-SIMA. Describe any changes made to build system: N/A Describe any changes made to the namelist: N/A List any changes to the defaults for the input datasets (e.g. boundary datasets): N/A List all files eliminated and why: D libraries/mct - Remove unused git submodule. List all files added and what they do: N/A List all existing files that have been modified, and describe the changes: (Helpful git command: `git diff --name-status development...<your_branch_name>`) M .gitmodules M ccs_config M cime M components/cdeps M components/cice M components/cism M components/clm M components/cmeps M components/mosart M components/rtm M libraries/FMS M libraries/parallelio M tools/CUPiD - Update git submodules to match/exceed the versions used in cam6_4_106 If there are new failures (compared to the `test/existing-test-failures.txt` file), have them OK'd by the gatekeeper, note them here, and add them to the file. If there are baseline differences, include the test and the reason for the diff. What is the nature of the change? Roundoff? derecho/intel/aux_sima: ALL PASS derecho/gnu/aux_sima: SMS_Ln9.ne3pg3_ne3pg3_mg37.FCAM4.derecho_gnu.cam-outfrq_se_cslam_analy_ic (Overall: DIFF) SMS_Ln9.ne3pg3_ne3pg3_mg37.FCAM7.derecho_gnu.cam-outfrq_se_cslam_analy_ic (Overall: DIFF) SMS_Ln9.ne3pg3_ne3pg3_mg37.FHS94.derecho_gnu.cam-outfrq_se_cslam (Overall: DIFF) SMS_Ln9.ne3pg3_ne3pg3_mg37.FKESSLER.derecho_gnu.cam-outfrq_se_cslam (Overall: DIFF) SMS_Ln9.ne3pg3_ne3pg3_mg37.FTJ16.derecho_gnu.cam-outfrq_se_cslam (Overall: DIFF) - Answer changes due to updated externals. If this changes climate describe any run(s) done to evaluate the new climate in enough detail that it(they) could be reproduced: CAM-SIMA date used for the baseline comparison tests if different than latest:
nusbaume
requested changes
Aug 6, 2025
nusbaume
left a comment
nusbaume
left a comment
Collaborator
There was a problem hiding this comment.
I have some hopefully minor cleanup and comment/wording requests, but otherwise it all looks good to me. Thanks @kuanchihwang!
…ndencies" This reverts commit 2c69c06. ESMCI/CIME #4818 has been merged and tagged as cime6.1.113 in the upstream. This workaround can now be removed.
nusbaume
approved these changes
Aug 12, 2025
nusbaume
left a comment
nusbaume
left a comment
Collaborator
There was a problem hiding this comment.
Everything looks great to me now. Thanks again @kuanchihwang!
mgduda
approved these changes
Aug 13, 2025
mgduda
left a comment
mgduda
left a comment
Collaborator
There was a problem hiding this comment.
Apologies for taking so long to provide a review!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 refactors MPAS dynamical core in preparation for unit testing. Specifically,
dyn_{comp,coupling,grid}modules have been converted to submodules to avoid further circular dependency issues. Now, thedyn_{comp,coupling,grid}modules only contain public interfaces and variables. This is analogous to the header files (*.h) in C. Thedyn_{comp,coupling,grid}_implsubmodules contain the actual implementations for the respective modules. This is analogous to the source files (*.c) in C.dyn_exchange_constituent_statessubroutine can be moved todyn_coupling, while thedyn_inquire_mesh_dimensionssubroutine as well as its associated variables can be moved todyn_grid. This relocation better reflects their functional context.dyn_{,mpas_}proceduremodules in preparation for unit testing. Unit tests will then focus on these two modules in the future.The points 1. and 2. are the solution in response to a previous review comment by @nusbaume.
Describe any changes made to the build system:
Describe any changes made to the namelist:
None
List any changes to the defaults for the input datasets (e.g., boundary datasets):
None
List all files eliminated and why:
None
List all files added and what they do:
List all existing files that have been modified, and describe the changes:
Regression tests:
No changes to any existing tests. All tests pass with respect to the last baseline,
sima0_07_000.