Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revise framework #1283

Merged
merged 13 commits into from
Jan 14, 2021
Merged

Conversation

Hallberg-NOAA
Copy link
Collaborator

MOM6: +Revise framework code

Restructured and revised code in the MOM6 framework directory to separate
MOM6-specific code from modules with direct calls to FMS or mpp routines. All
answers and output files are bitwise identical, except for the ocean_geometry.F90 file
where incorrect dimensional rescaling of areas was corrected. Although there are
new interfaces and new optional arguments, and this PR includes changes that
exercise these changes, the contents of the new framework directory can be
swapped with previous versions of the code to give identical answers and all
output files.

The commits in this PR include:

  Added the new file MOM_domain_init.F90, with a copy of MOM_domains_init, to
facilitate the separation of the framework code into FMS-specific and
FMS-independent directories, and use this new module in MOM.F90 and
MOM_ice_shelf.F90.  There is also a copy of MOM_domains_init still in
MOM_domains.F90, so other modules, like SIS2, will continue to work.  Some of
the previous contents of MOM_domains_init have been transferred into the new
publicly visible routines create_MOM_domain, MOM_thread_affinity_set and
set_MOM_thread_affinity.  Also removed several module use statements for
MOM_domains_init that are not needed.  All answers and output files are bitwise
identical, but there are new public interfaces.
  Moved rotated_write_field from MOM_transform_FMS.F90 to MOM_io.F90 and renamed
it to MOM_write_field, with the mpp_domain argument replaced with a MOM_domain
argument.  Also changed the calls in save_restart to reflect these changes.  All
answers and output files are identical.
  Use calls to MOM_write_field in write_ocean_geometry_file, instead of calls
write_field.  Also added missing dimensional rescaling factors to the two area
fields Ah and Aq, which will cause the output fields in the ocean_geometry.nc
files to change, but they are now invariant to the choice of dimensional
rescaling, whereas previously they had not been.  All answers and output are
otherwise bitwise identical, and even ocean_geometry.nc is identical if
L_RESCALE_POWER = 0.
  Added the new routines deallocate_MOM_domain, deallocate_domain_contents and
get_layout_extents to standardize the clean-up of memory associated with
MOM_domains, provide an interface for obtaining information about the global
grid decomposition and limit the dependencies on mpp functions to calls that go
through the MOM framework directory.  All answers are bitwise identical,
although there are new public interfaces.
  Renamed rotated_mpp_chksum to rotated_field_chksum and moved the routines
wrapped by this overloaded interface from MOM_transform_FMS.F90 to
MOM_checksums.F90.  Also provided access to mpp_chksum as field_chksum via
MOM_coms.F90.  Both of these are steps to clean up the MOM6 framework code and
reduce the direct use of mpp routines in the rest of the MOM6 code.  All answers
are bitwise identical, but there are effectively new interfaces, and one
existing interface was renamed.
  Added an explicit interface for field_exists to MOM_io.F90, and added a new
optional argument of a MOM_domain_type to field_exists.  All answers are bitwise
identical, and all previous calls still work exactly as before, but there is a
new optional argument.
  Added the new interface get_domain_components to MOM_domains.F90 to return the 1-d domains that
are make up a 2-d domain, with overloaded variants working on MOM_domain_type or
domain2D arguments.  The MOM_domains module also now provides access to the
domain2D type.  All answers are bitwise identical.
  Created the new module MOM_io_wrapper from the contents of MOM_io, so that
there are two separate modules, one of which use MOM-specific calls and
structures, and another that directly calls or wraps the mpp and FMS I/O
interfaces.  All of the previous interfaces that were accessible via MOM_io
are still being served from the this module, so no changes are needed outside
of these two modules.  All answers are bitwise identical.
  Added an explicit interface for open_file to MOM_io_wrapper.F90.  This version
only includes the optional arguments that are actually used in the MOM6 or SIS2
code, but adds a new optional MOM_domain_type argument.  The optional arguments
to mpp_open that are not being carried over to the new MOM interface pertain to
archaic file formats and will never be used in MOM6.  All answers are bitwise
identical.
@codecov-io
Copy link

codecov-io commented Jan 11, 2021

Codecov Report

Merging #1283 (77d0cbe) into dev/gfdl (42a9eaf) will decrease coverage by 0.25%.
The diff coverage is 31.04%.

Impacted file tree graph

@@             Coverage Diff              @@
##           dev/gfdl    #1283      +/-   ##
============================================
- Coverage     46.08%   45.82%   -0.26%     
============================================
  Files           214      227      +13     
  Lines         69399    71552    +2153     
============================================
+ Hits          31984    32791     +807     
- Misses        37415    38761    +1346     
Impacted Files Coverage Δ
...g_src/external/GFDL_ocean_BGC/FMS_coupler_util.F90 0.00% <0.00%> (ø)
...fig_src/external/GFDL_ocean_BGC/generic_tracer.F90 0.00% <0.00%> (ø)
...c/external/GFDL_ocean_BGC/generic_tracer_utils.F90 0.00% <0.00%> (ø)
config_src/external/ODA_hooks/kdtree.f90 0.00% <0.00%> (ø)
config_src/external/ODA_hooks/ocean_da_core.F90 0.00% <0.00%> (ø)
config_src/external/ODA_hooks/ocean_da_types.F90 0.00% <0.00%> (ø)
config_src/external/ODA_hooks/write_ocean_obs.F90 0.00% <0.00%> (ø)
config_src/solo_driver/MESO_surface_forcing.F90 0.00% <0.00%> (ø)
config_src/solo_driver/user_surface_forcing.F90 0.00% <0.00%> (ø)
src/ALE/P1M_functions.F90 0.00% <0.00%> (ø)
... and 232 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 91b6a15...f7beef4. Read the comment docs.

  Added doxygen comments describing two arguments to get_layout_extents.  All
answers are bitwise identical.
@Hallberg-NOAA
Copy link
Collaborator Author

Pipeline testing for this PR can be found at https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/11841 .

@marshallward
Copy link
Collaborator

The pipeline is failing on a segmentation fault:

Program received signal SIGSEGV: Segmentation fault - invalid memory reference.

This fail is happening under the OM4_025 test using the GNU REPRO build.

The GNU Debug build appears to pass for me.

Unfortunately our repro build seems to have been built without -g, so I'm unable to probe very far here, but what I can see appears to be related to a memory allocation:

I will keep looking into this but feel free to do your own inspection, @Hallberg-NOAA.

@adcroft
Copy link
Collaborator

adcroft commented Jan 13, 2021

I see fails over more experiments with both gnu and intel:

	modified:   regressions/ice_ocean_SIS2/Baltic_ALE_z_offline_tracers/seaice.stats.intel
	modified:   regressions/ice_ocean_SIS2/Baltic_OM4_025/seaice.stats.intel
	modified:   regressions/ice_ocean_SIS2/OM4_05/seaice.stats.gnu
	modified:   regressions/ice_ocean_SIS2/OM4_05/seaice.stats.intel
	modified:   regressions/ice_ocean_SIS2/OM_1deg/seaice.stats.gnu
	modified:   regressions/ice_ocean_SIS2/OM_1deg/seaice.stats.intel
	modified:   regressions/ice_ocean_SIS2/SIS2/ocean.stats.gnu
	modified:   regressions/ice_ocean_SIS2/SIS2/seaice.stats.gnu
	modified:   regressions/ice_ocean_SIS2/SIS2/seaice.stats.intel
	modified:   regressions/ice_ocean_SIS2/SIS2_bergs_cgrid/seaice.stats.gnu
	modified:   regressions/ice_ocean_SIS2/SIS2_bergs_cgrid/seaice.stats.intel
	modified:   regressions/ice_ocean_SIS2/SIS2_cgrid/ocean.stats.gnu
	modified:   regressions/ice_ocean_SIS2/SIS2_cgrid/seaice.stats.gnu
	modified:   regressions/ice_ocean_SIS2/SIS2_cgrid/seaice.stats.intel

@marshallward
Copy link
Collaborator

Here is a backtrace of the error:

#0  0x16698df in ???
	at /home/abuild/rpmbuild/BUILD/glibc-2.26/nptl/../sysdeps/unix/sysv/linux/x86_64/sigaction.c:0
#1  0xdc2eb6 in __mom_domains_MOD_clone_md_to_md
	at /lustre/f2/dev/gfdl/Marshall.Ward/CI/Gaea-stats-MOM6-examples/MOM6-examples/src/MOM6/src/framework/MOM_domains.F90:1802
#2  0xd7813e in __ice_model_mod_MOD_ice_model_init
	at /lustre/f2/dev/gfdl/Marshall.Ward/CI/Gaea-stats-MOM6-examples/MOM6-examples/src/SIS2/src/ice_model.F90:2073
#3  0x950aad in coupler_init
	at /lustre/f2/dev/gfdl/Marshall.Ward/CI/Gaea-stats-MOM6-examples/MOM6-examples/src/coupler/coupler_main.F90:1809
#4  0x9457e7 in coupler_main
	at /lustre/f2/dev/gfdl/Marshall.Ward/CI/Gaea-stats-MOM6-examples/MOM6-examples/src/coupler/coupler_main.F90:611
#5  0x952895 in main
	at /lustre/f2/dev/gfdl/Marshall.Ward/CI/Gaea-stats-MOM6-examples/MOM6-examples/src/coupler/coupler_main.F90:313

  Corrected a bug that was incorrectly using the IO_LAYOUT to set LAYOUT when
the IO_LAYOUT uses non-default values greater than 1.  All answers are bitwise
identical in any cases that worked.
@marshallward
Copy link
Collaborator

@marshallward
Copy link
Collaborator

marshallward commented Jan 14, 2021

@marshallward marshallward merged commit 0bd16f4 into mom-ocean:dev/gfdl Jan 14, 2021
@Hallberg-NOAA Hallberg-NOAA deleted the revise_framework branch July 30, 2021 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants