Moving Nest Code Restructure and Port to fms2_io

[wramstrom]

Description

1. Improve modularization of moving nest code (see also PR in fv3atm)
2. Code cleanup for moving nest and remove debugging outputs
3. Upgrade to fms2_io

The moving nest code directory has been moved from atmos_cubed_sphere up one level to FV3. This directory restructuring removes all references to physics code due to moving nest in the dynamic core code in atmos_cubed_sphere. All calls into moving nest functionality are now performed from FV3/atmos_model.F90. atmos_cubed_sphere code no longer depends on moving nest code, or calls its subroutines.

Debugging print statements and subroutines that would print out data structure contents have been removed. While these were disabled during real-time and operational runs, some overhead in executing conditional statements remained. A minor performance improvement may be noticed. In addition, code clarity is now improved by removing these lines and subroutines.

Port to fms2_io was performed. The moving nest code no longer depends on the legacy fms_io library. Operational moving nest code only uses fms2_io for reading the high-resolution static files. This read code performs the file access to read from disk only on the head node of the PElist, then broadcasts the data to the other PEs. This may permit improved performance over the prior method, where each nest PE read the file from disk. Additional debugging code to dump netCDF files of model variables is normally commented out, but can be enabled during development. This debugging code makes use of the fms2_io domain decomposed write functionality.

Fixes #182

Related to PR 610 which creates the new FV3/moving_nest directory.
NOAA-EMC/ufsatm#610

How Has This Been Tested?

Tests were performed on Orion. Moving nest code showed bitwise identical results as older runs from the code cleanup and fms2_io. Some additional parts of the merge in other areas cause differences in forecast values, buy these are expected according to Bin Liu.

Full regression suite was run on Orion, with all tests running to completion. 148 of the tests passed fully. The remaining tests are 13 of the HAFS tests, which ran to completion but showed expected differences in numerical outputs.

Checklist:

Please check all whether they apply or not

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• Any dependent changes have been merged and published in downstream modules

[lharris4]

If all of the moving-nest specific code is removed from the dycore, then this commit is good modulo the one very minor suggestion I have. Thank you for working on this.

[lharris4]

May want to set to .false. or be triggered by some runtime option.

[wramstrom]

This suggestion makes sense. I think that adding a namelist option to turn on the timing code makes sense, so that we could do it without a recompile. @BinLiu-NOAA and others, do you agree?

[wramstrom]

@lharris4 I have added a namelist flag, defaulting to false, that enables the detailed performance timing calls. It parallels fv_debug, and is called fv_timers.

In looking at the code for this change, I noticed that there are lines of moving nest code in tools/fv_diagnostics.F90 to reinitialize zsurf after nest moves, and to register/produce lat/lon variables that reflect the nest motion, called grid_mlat, grid_mlon, grid_mlatt, and grid_mlont, which I had forgotten to mention. Let me know your thoughts on these code additions.

[lharris4]

Hi, Bill. Thanks for reminding me of this. I don't object to having this capability in fv_diagnostics, which is reasonably unobtrusive and mostly well-encapsulated. It may be useful for other purposes too, like the immersed-boundary method or for dynamic regridding. Lucas

…

On Thu, Dec 8, 2022 at 2:26 PM William Ramstrom ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In tools/fv_grid_tools.F90 <#227 (comment)> : > @@ -605,8 +605,8 @@ subroutine init_grid(Atm, grid_name, grid_file, npx, npy, npz, ndims, nregions, ! Setup timing variables logical, save :: first_time = .true. - integer, save :: id_timer1, id_timer2, id_timer3, id_timer3a, id_timer4, id_timer5, id_timer6, id_timer7, id_timer8 - logical :: use_timer = .false. ! Set to True for detailed performance profiling + integer, save :: id_timer1, id_timer2, id_timer3, id_timer3a, id_timer3b, id_timer4, id_timer5, id_timer6, id_timer7, id_timer8 + logical :: use_timer = .True. ! Set to True for detailed performance profiling @lharris4 <https://github.com/lharris4> I have added a namelist flag, defaulting to false, that enables the detailed performance timing calls. It parallels fv_debug, and is called fv_timers. In looking at the code for this change, I noticed that there are lines of moving nest code in tools/fv_diagnostics.F90 to reinitialize zsurf after nest moves, and to register/produce lat/lon variables that reflect the nest motion, called grid_mlat, grid_mlon, grid_mlatt, and grid_mlont, which I had forgotten to mention. Let me know your thoughts on these code additions. — Reply to this email directly, view it on GitHub <#227 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AMUQRVCE2Z6JN2IMVVWHSZDWMIY55ANCNFSM6AAAAAASRF6WF4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[junwang-noaa]

@wramstrom I still see fms_io dependency in: tools/fv_restart.F90

https://github.com/hafs-community/GFDL_atmos_cubed_sphere/blob/feature/mn_cleanup_202211/tools/fv_restart.F90#L171

Can we use fms2_io in the following lines:

https://github.com/hafs-community/GFDL_atmos_cubed_sphere/blob/feature/mn_cleanup_202211/tools/fv_restart.F90#L288-L292

[wramstrom]

@junwang-noaa That code is part of the dycore, not the moving nest; I'm not familiar with that section. It looks from the log that @bensonr made some changes over the summer to this file to prepare it for fms2_io, and from the comments that perhaps that last fms_io call can be removed. I'd like to have @bensonr weigh in with his opinion on this.

[laurenchilutti]

That section of fv_restart was intentionally left as a redundancy between fmsio and fms2io in the cases where other model components may still be using fms_io and would need this old fmsio set_filename_appendix called. I can discuss removing this with @bensonr and some of the fms2io developers. But I do not think Bill needs to remove it in this PR.

[junwang-noaa]

@wramstrom @laurenchilutti Thanks for the explanation!

[jkbk2004]

all regression tests are done on ufs wm pr 1518. please, go ahead to start merging in this pr.

[lharris4]

This is a nice addition; it could be extended in the future to add more fine-grained timers.

[bensonr]

Actually, the granularity of output for MPP clocks is determined by an FMS namelist variable that works with the various timer levels specified in the clock_id to determine at what level a clock will be output.

[lharris4]

Looks good to me. Thanks.

[jkbk2004]

@lharris4 thanks for the approval! @laurenchilutti @bensonr could you go ahead to merge the pr?

[bensonr]

Actually, the granularity of output for MPP clocks is determined by an FMS namelist variable that works with the various timer levels specified in the clock_id to determine at what level a clock will be output.