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

Add fms2-io to MOM restart interfaces #1165

Merged
merged 68 commits into from
Mar 26, 2021
Merged

Add fms2-io to MOM restart interfaces #1165

merged 68 commits into from
Mar 26, 2021

Conversation

wrongkindofdoctor
Copy link
Contributor

@wrongkindofdoctor wrongkindofdoctor commented Jul 21, 2020

This PR contains the upgrades to save_restart and restore_state, and their dependencies required to use the fms2-io interfaces. Major changes include:

  • making restore_state and save_restart wrapper routines with optional use_fms2 and write_ic arguments that, when present and .true., call the routines that contain the appropriate fms2-io interfaces
  • adding the logical argument use_fms2=.true. to save_restart and restore_state calls in MOM6 code
  • making create_file an interface with routines that wrap calls to mpp-io and fms2-io routines
  • adding the module MOM_axis.F90 with helper routines for defining, querying, reading, and writing axes using fms2-io
  • adding the module MOM_read_data_fms2.F90 with MOM_read_data routines that wrap fms2-io read_data calls
  • adding the module MOM_write_field_fms2.F90 with write_field routines that wrap fms2-io write_data calls
  • note that the MOM_read_data and MOM_write_field interfaces that are called outside of the framework modules will be updated in a separate PR.

wrongkindofdoctor and others added 30 commits October 8, 2018 09:58
merge in latest MOM updates
* GFDL_MOM6/dev/gfdl: (220 commits)
  Update cache dir for /lustre/f2
  Update cache dir for /lustre/f2
  Calculate height-related diagnostics in Z
  Set coord_adapt and coord_slight parameters in H
  Corrected comments in build_zlike_column
  +Pass max_depth to initialize_regridding in Z
  Corrected comments in build_sigma_column
  Combined scaling factors in build_adapt_column
  Clarified comments in Idealized_Hurricane
  +Recast MOM_diag_to_Z to work in units of Z
  +Recast MOM_ALE_sponge to work in units of Z
  +Added m_to_Z arg to horiz_interp_and_extrap_tracer
  Find energetic_PBL column height changes in Z
  +Find diapyc_energy_req column height changes in Z
  Use local variables to rescale in MOM_Point_Accel
  Recast MOM_sum_output to work in units of Z
  Simplify MOM_diagnostics code
  +Rescale depth inside of MEKE_lengthScales_0d
  +Add conversion argument to register_static_field
  Rescale values reported by PointAccel
  ...
Merge in latest dev/gfdl updates
Merge in latest MOM6 dev/gfdl updates
Merge in latest MOM dev/gfdl updates
Merge in latest MOM6 updates
Merge in latest dev/gfdl updates
Merge in latest dev/gfdl updates
merge latest updates into dev/gfdl
Merge in latest dev/gfdl commits
Merge in latest dev/gfdl updates
Merge in latest dev/gfdl updates
merge in latest dev/gfdl changes
merge in latest dev/gfdl updates
Merge in latest MOM6 updates
Merge in latest dev/gfdl updates
Merge in latest round of dev/gfdl updates
merge in latest dev/gfdl updates
Merge in latest dev/gfdl updates
Merge in latest dev/gfdl updates
Merge in latest dev/gfdl updates
Merge in latest updates from dev/gfdl
Merge in latest dev/gfdl updates
Merge in updates to remap_all_state_vars
* TC4 integration into test suite

This patch renames the tc4 test to activate it in the test suite.  It
also modifies the Makefile to build the input field test scripts.

It also modifies the Python build scripts to be PEP8-conformant.

We temporarily disable tc4 in the restart tests, since they currently
fail.  This needs to be addressed before we can merge this into the main
branch.

The patch does not enable the necessary Python modules for running on
Travis, that will also be addressed later.

* Travis python support; tc4 Makefile

The custom TC4 Makefile has been added (oops), and the presumed Python
Ubuntu packages have been added for Travis.

* Verify ENABLE_THERMODYNAMICS is True before posting C_p diagnostic

* Make tc4 faster

* remove trailing whitespace

* add unit scaling

* fix restart fail for tc4 and some cleanup

* remove trailiny ws

* Enable tc4.restart test

* +Pass timeesteps to tracer diagnostics in [T]

  Pass timeesteps to the tracer diagnistics routines post_tracer_diagnostics and
postALE_tracer_diagnostics and to adiabatic in units of [T}.  All answers are
bitwise identical.

* +Rescaled tracer advective flux diagnostics

  Rescaled the internal units of the tracer advective flux diagnostics to units
of [conc H L2 T-1] for code simplicity and dimensional consistency testing.
Also corrected the units of some tracer fluxes as documented in comments and
commented out unused elements of the tracer_type.  All answers are bitwise
identical.

* +Pass timesteps to ALE_main in [T]

  Pass the timesteps to ALE_main, ALE_main_offline, and ALE_main_accelerated in
units of [T] for code simplicity and dimensional consistency testing.  This also
includes the rescaling of remapping-driven tracer tendencies.  All answers and
diagnostics are bitwise identical.

* +Pass timesteps to tracer column_physics in [T]

  Pass timesteps to the various tracer column_physics routines in [T] for
dimensional consistency testing.  Also added a new unit_scale_type argument to
these routines.  All answers are bitwise identical, but there are minor
interface changes to 13 subroutines.

* +Pass timesteps to applyTracerBoundaryFluxesInOut in [T]

  Pass timesteps to applyTracerBoundaryFluxesInOut in [T], and use units of
[T-1] for internal source and decay rates for the oil tracer and in fluxes of
CFCs.  Also modified extract_offline_main to return timesteps as real values
with units of [T].  Also there is a new unit_scale_type argument to
register_oil_tracer.   All answers in the MOM6_examples test cases and
regression tests are bitwise identical.

* Simplified expressions in MOM_PointAccel

  Simplified expressions inside of MOM_PointAccel, taking into account that all
velocities use the same units of [L T-1].  All answers are bitwise identical.

* Corrected dimensional epsilons in downscaling

  Added distinct negligible volumes, face areas, horizonal areas and lengths
with proper dimensional rescaling in the downsample field routines.  With these
changes, downscaled diagnostics should now pass the dimensional rescaling tests,
whereas previously there would have been a problem when the numbers used to
represent lengths are smaller than about 1e-8 times their MKS values.  All
answers are bitwise identical without dimensional rescaling.

* Simplified expressions in MOM_offline_aux

  Simplified expressions in distribute_residual_uh_barotropic.  All answers are
bitwise identical.

* Revised wave_speed to return speed in [L T-1]

  Revised wave_speed to return the internal wave speed in units of [L T-1] and
to use mono_N2_depth in units of [Z] for code simplification and expanded
dimensional consistency testing.  Also revised the internal units of some
related diagnostics in calculate_diagnostic_fields.  All answers and diagnostics
are bitwise identical.

* Rescaled internal variables in wave_speed

  Rescale internal calculations in wave_speed and wave_speeds for greater
robustness and dimensional consistency testing.  All answers are bitwise
identical and pass dimensional scaling tests.

* +Changed the units of minimum_forcing_depth to [H]

  Changed the units of minimum_forcing_depth passed to applyBoundaryFluxesInOut
and applyTracerBoundaryFluxesInOut to [H].  All answers are bitwise identical.

* Correction of documented units in comments

  Corrected some units in comments and eliminated some unused variables.
All answers are bitwise identical.

* Adiabatic clock ID bugfix

This patch fixes an initialization bug of the diabatic timer, which was
being used to measure adiabatic time but was never initialized if the
experiment was configured as adiabatic.

We fix this by introducing a separate timer for the adiabatic solver.
Although we could have reused the diabatic timer, the addition of a new
variable should not add any overhead on modern compilers.

* Corrected an OMP declaration

  Added a variable to an OMP declaration.  All answers are bitwise identical,
and a recently added compile-time error with openMP was fixed.

* Update MOM.F90

Fixed Alistair's embarrassing error.

* Dimensional rescaling in MOM_open_boundary.F90

  Added rescaling for dimensional consistency testing in MOM_open_boundary.F90,
including splitting variables with different units that had previously shared
the same variable and adding more extensive documentation of variables.  Also
changed the dimensions of the timesteps passed to radiation_open_bdry_conds and
update_segment_tracer_reservoirs to [T] and added vertical_grid_type and
unit_scale_type arguments to open_boundary_init and open_boundary_test_extern_h.
All answers are bitwise identical, although some probably bugs have been noted
in comments and there are new or altered arguments to several routines.

* (*)Fixed invariance bugs in MOM_open_boundary.F90

  Corrected dimensional consistency bugs in update_segment_tracer_reservoirs and
horizontal indexing and related bugs in gradient_at_q_points with oblique_grad
OBCs.  These will both change answers in test cases that use some open boundary
condition options, but not in any of the MOM6-examples test cases.
@wrongkindofdoctor
Copy link
Contributor Author

@marshallward Yes, this is ready for review. If you feel some changes are unnecessary with the configuration file, you can modify them as you see fit; my intent is to sync with FMS beta releases. I have implemented the fms2 selection in save_restart using the wrapper routine instead of an interface as requested, so it would help if you provided an example.

@marshallward
Copy link
Collaborator

There should not be any changes outside of framework. Removing the use_fms2 argument and controlling this inside of save_restart (hard-coded for now) should work, yes?

@wrongkindofdoctor
Copy link
Contributor Author

@marshallward Would use_fms2 =.true. set as module variable in MOM_restart rather than a flag suffice?

@marshallward
Copy link
Collaborator

marshallward commented Aug 18, 2020

Yes, I think a module parameter would be fine. We can work out something more elegant on our end if needed.

@adcroft
Copy link
Collaborator

adcroft commented Aug 18, 2020 via email

@wrongkindofdoctor
Copy link
Contributor Author

@marshallward @adcroft I think we need to meet offline to flesh out the requirements.

wrongkindofdoctor and others added 5 commits August 24, 2020 16:31
Sync with NOAA-GFDL dev/gfdl
…ons of the routines with the fms-io or fms2-io interfaces

added module use statments for fms2_io and MOM_io helper routines to MOM_restart
added use_fms2=.true. arguments to save_restart and restore_state calls
added write_ic=.true. to the save_restart call in MOM.F90

added module MOM_axis with routines to define and register axes and their metadata
added module MOM_read_data_fms2.F90 with wrappers for fms2_io read_data interfaces and required routines
added module MOM_write_field_fms2.F90 with wrappers for fms2_io write_data interfaces
updated module use statments in MOM_io and MOM_restart to reference routines in MOM_read_data_fms2, MOM_write_field_fms2, and MOM_axis
made write_field and create_file interfaces in MOM_io
added create_file routines to MOM_io that accept file names or file objects to create/overwrite netcdf files that will be written to via write_field calls

fixed compile-time errors
added new MOM_read_data routines to MOM_read_data interface in MOM_io
added placeholder call for new write_field routines to MOM_io

changed use_fms2 to a required first argument in save_restart_fms2 and restore_state_fms2
changed write_ic to a required argument in write_initial_conditions

fixed the layer and interface checks in MOM_get_diagnostic_axis_data

commented out manual checksum registration in save_restart_fms2 so that internal fms2-io checksum computation is used

added checks for time units to restore_state and save_restart
added logic to make the restart time 1 to save_restart_fms2 if there is an abnormally large value passed to the routine
added interface routine file_exists_FMS2 that uses the fms2_io file_exists call

added subroutine get_num_restart_files to MOM_restart that searches for known variants of the input file names and returns then number of restart files available for querying, and the optional list of filepaths
added loop to search the files for all mandatory variables in the list of file paths returned to by cal to get_num_restart_files
added calls to get the variable dimension names and pass them as arguments to register_restart_field in restore_state_fms2

removed exit from inner variable loop in restore_state_fms2
code cleanup

moved missing_fields=0 outside of the CS loop in restore_state_fms2

moved missing_fields=0 outside of the CS loop in restore_state_fms2

converted save_restart and restore_state to interface that call versions of the routines with the fms-io or fms2-io interfaces
added module use statments for fms2_io and MOM_io helper routines to MOM_restart
added use_fms2=.true. arguments to save_restart and restore_state calls
added write_ic=.true. to the save_restart call in MOM.F90

added module MOM_axis with routines to define and register axes and their metadata
added module MOM_read_data_fms2.F90 with wrappers for fms2_io read_data interfaces and required routines
added module MOM_write_field_fms2.F90 with wrappers for fms2_io write_data interfaces
updated module use statments in MOM_io and MOM_restart to reference routines in MOM_read_data_fms2, MOM_write_field_fms2, and MOM_axis
made write_field and create_file interfaces in MOM_io
added create_file routines to MOM_io that accept file names or file objects to create/overwrite netcdf files that will be written to via write_field calls

fixed compile-time errors
added new MOM_read_data routines to MOM_read_data interface in MOM_io
added placeholder call for new write_field routines to MOM_io

changed use_fms2 to a required first argument in save_restart_fms2 and restore_state_fms2
changed write_ic to a required argument in write_initial_conditions

fixed the layer and interface checks in MOM_get_diagnostic_axis_data

commented out manual checksum registration in save_restart_fms2 so that internal fms2-io checksum computation is used

added checks for time units to restore_state and save_restart
added logic to make the restart time 1 to save_restart_fms2 if there is an abnormally large value passed to the routine
added interface routine file_exists_FMS2 that uses the fms2_io file_exists call

added subroutine get_num_restart_files to MOM_restart that searches for known variants of the input file names and returns then number of restart files available for querying, and the optional list of filepaths
added loop to search the files for all mandatory variables in the list of file paths returned to by cal to get_num_restart_files
added calls to get the variable dimension names and pass them as arguments to register_restart_field in restore_state_fms2

removed exit from inner variable loop in restore_state_fms2
code cleanup

removed test workflow directory

moved missing_fields=0 outside of the CS loop in restore_state_fms2

converted save_restart and restore_state to interface that call versions of the routines with the fms-io or fms2-io interfaces
added module use statments for fms2_io and MOM_io helper routines to MOM_restart
added use_fms2=.true. arguments to save_restart and restore_state calls
added write_ic=.true. to the save_restart call in MOM.F90

added module MOM_axis with routines to define and register axes and their metadata
added module MOM_read_data_fms2.F90 with wrappers for fms2_io read_data interfaces and required routines
added module MOM_write_field_fms2.F90 with wrappers for fms2_io write_data interfaces
updated module use statments in MOM_io and MOM_restart to reference routines in MOM_read_data_fms2, MOM_write_field_fms2, and MOM_axis
made write_field and create_file interfaces in MOM_io
added create_file routines to MOM_io that accept file names or file objects to create/overwrite netcdf files that will be written to via write_field calls

fixed compile-time errors
added new MOM_read_data routines to MOM_read_data interface in MOM_io
added placeholder call for new write_field routines to MOM_io

changed use_fms2 to a required first argument in save_restart_fms2 and restore_state_fms2
changed write_ic to a required argument in write_initial_conditions

fixed the layer and interface checks in MOM_get_diagnostic_axis_data

commented out manual checksum registration in save_restart_fms2 so that internal fms2-io checksum computation is used

added checks for time units to restore_state and save_restart
added logic to make the restart time 1 to save_restart_fms2 if there is an abnormally large value passed to the routine
added interface routine file_exists_FMS2 that uses the fms2_io file_exists call

added subroutine get_num_restart_files to MOM_restart that searches for known variants of the input file names and returns then number of restart files available for querying, and the optional list of filepaths
added loop to search the files for all mandatory variables in the list of file paths returned to by cal to get_num_restart_files
added calls to get the variable dimension names and pass them as arguments to register_restart_field in restore_state_fms2

removed exit from inner variable loop in restore_state_fms2
code cleanup

removed test workflow directory

removed white space

updated FMS tag in .testing Makefile
fixed argument comments to use doxygen style in MOM_write_field_fms2

code cleanup

removed \TODO from MOM_restart
fixed documentation for module variables in MOM_read_data_fms2 and MOM_write_field_fms2

removed doxygenized TODO statements

removed new MOM_read_data routines from interface, and commented out calls in MOM_io
fixed doxygen definitions

more doxygen fixes

changed save_restart and restore_state to wrapper routines with opitional use_fms2 and write_ic arguments
added use_fms2=.true. and write_ic=.true. to save_restart and restore_state calls

removed the error messages from append_substring
tried reordering the write_ic and use_fms2 checks in save_restart to fix error with invalid memory reference in the MOM.F90 call to save_restart-write_initial_conditions
…ing the use_fms2 and write_ic flags if present to avoid invalid memory reference error

added str_len argument to register_variable_attribute calls

added support to for rotated fields to write_initial_conditions and save_restart_fms2

removed whitespace
removed use_fms2 optional arguments in save_restart, restore_state, and create_file
made use_fms2 a MOM_restart module variable

Update MOM_state_initialization.F90

Remove space.

Update MOM_ice_shelf.F90

Add space.

Update MOM_driver.F90

Add space

Update mom_surface_forcing_mct.F90

remove whitespace

Update ocean_model_MOM.F90

Add space

Update mom_ocean_model_nuopc.F90

Add space

Update MOM_surface_forcing.F90

Add space

Changes needed work ESM4 to run with new io
allocate(file_times(dim_unlim_size))
call read_data(fileobj,trim(dim_unlim_name), file_times)

do i=1,dim_unlim_size
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that get_time_index should include proper error handling for the case when the time being requested is outside of the range of times spanned by the time index of the input file.

@sanAkel
Copy link
Collaborator

sanAkel commented Nov 25, 2020

@marshallward , @wrongkindofdoctor a question from myself and @mathomp4

Once this PR is merged, does it

  • require FMS 2020.03?
  • or it can use FMS 2020.03?

Knowing this or any other requirements would help us update since we the same FMS in MOM6 and FV3. Thanks a lot!

@adcroft
Copy link
Collaborator

adcroft commented Nov 25, 2020

I think it does require the new FMS (for single column mode there's a change in MOM6 required). We've been sitting on this because downstream users are still recovering from the last time we changed the version of FMS. There is an answer change associated with i/o due to a bug fix in FMS which affects a few setups. While we know how to recover answers with .03 there's another release in which it will be easier to disable the fix (if needed, via the namelist), coming in .04. Since these require changes to either build or run scripts, as well as MOM6, we figured it better to bundle the next two changes together and do so in a focused PR with nothing else in it. This is happening and it's in our hands now - it's been in the pipe for a long time but we're cognizant of how painful it is downstream and are trying minimize impact by combining the changes.

@mathomp4
Copy link
Collaborator

@adcroft I'd been avoiding it only because syncing up forks is so annoying. But if all I need to do is target getting, say, 2020.04 in, that might make things easier (I have to make sure we change the constants file in all our releases).

And @sanAkel Did we make a PR for changing all the call flush() to flush() in FMS? If not, we/I might want to

@sanAkel
Copy link
Collaborator

sanAkel commented Nov 25, 2020

I think it does require the new FMS (for single column mode there's a change in MOM6 required). We've been sitting on this because downstream users are still recovering from the last time we changed the version of FMS. There is an answer change associated with i/o due to a bug fix in FMS which affects a few setups. While we know how to recover answers with .03 there's another release in which it will be easier to disable the fix (if needed, via the namelist), coming in .04. Since these require changes to either build or run scripts, as well as MOM6, we figured it better to bundle the next two changes together and do so in a focused PR with nothing else in it. This is happening and it's in our hands now - it's been in the pipe for a long time but we're cognizant of how painful it is downstream and are trying minimize impact by combining the changes.

@adcroft Thanks for that heads up! I do use the single column mode, but straight off of MOM6-examples, @mathomp4 , that has nothing to do with anything in GEOS.

@sanAkel
Copy link
Collaborator

sanAkel commented Nov 25, 2020

And @sanAkel Did we make a PR for changing all the call flush() to flush() in FMS? If not, we/I might want to

@mathomp4 I don't recall that you did. As much as I know, it is in our: GEOS-ESM, that's it.

@mathomp4
Copy link
Collaborator

mathomp4 commented Nov 25, 2020

And @sanAkel Did we make a PR for changing all the call flush() to flush() in FMS? If not, we/I might want to

@mathomp4 I don't recall that you did. As much as I know, it is in our: GEOS-ESM, that's it.

Okay.

So fair warning to @adcroft and @marshallward and all the GFDL folks. I'll probably make an Issue/PR to FMS and MOM5 MOM6 about this. Essentially we found that when using Intel Fortran on macOS that for some reason, Intel was getting confused by call FLUSH() calls. This is probably because the call FLUSH() subroutine is a Fortran extension. So we've been internally in GEOS and in our forks, converting all the call FLUSH() to use the Fortran Standard FLUSH() intrinsic (which I think every good Fortran compiler now should support) and that seems to be helping.

I'm going to ping Tom Clune to help me get the wording right for the Issue/PR so it sounds better than my "it made things work" explanation :)

@sanAkel
Copy link
Collaborator

sanAkel commented Nov 25, 2020

@mathomp4 you meant MOM6?

@mathomp4
Copy link
Collaborator

@mathomp4 you meant MOM6?

Sigh. Yeah. Too many years typing MOM5. :)

@marshallward
Copy link
Collaborator

marshallward commented Nov 26, 2020

I agree with this, flush() should be used over the call flush() vendor extension.


These are the instances I can find:

$ grep -ir "call \+flush" ../../{src,config_src}
../../src/diagnostics/MOM_sum_output.F90:  call flush_file(CS%fileenergy_nc)
../../src/diagnostics/MOM_PointAccel.F90:    call flush(file)
../../src/diagnostics/MOM_PointAccel.F90:    call flush(file)
../../src/framework/MOM_write_cputime.F90:    call flush(CS%fileCPU_ascii)
../../src/framework/MOM_write_cputime.F90:    call flush(CS%fileCPU_ascii)
../../src/framework/MOM_domains.F90:!$   call flush(6)
../../config_src/solo_driver/MOM_driver.F90:!$  call flush(6)

@mathomp4
Copy link
Collaborator

mathomp4 commented Nov 30, 2020

I agree with this, flush() should be used over the call flush() vendor extension.

These are the instances I can find:

$ grep -ir "call \+flush" ../../{src,config_src}
../../src/diagnostics/MOM_sum_output.F90:  call flush_file(CS%fileenergy_nc)
../../src/diagnostics/MOM_PointAccel.F90:    call flush(file)
../../src/diagnostics/MOM_PointAccel.F90:    call flush(file)
../../src/framework/MOM_write_cputime.F90:    call flush(CS%fileCPU_ascii)
../../src/framework/MOM_write_cputime.F90:    call flush(CS%fileCPU_ascii)
../../src/framework/MOM_domains.F90:!$   call flush(6)
../../config_src/solo_driver/MOM_driver.F90:!$  call flush(6)

This looks right to me:

git diff dev/gfdl --name-only
config_src/solo_driver/MOM_driver.F90
src/diagnostics/MOM_PointAccel.F90
src/framework/MOM_domains.F90
src/framework/MOM_write_cputime.F90

Would you like me to make a PR here and with FMS?

Okay. Here's the MOM6 PR: https://github.com/NOAA-GFDL/MOM6/pull/1264 and here is the FMS: NOAA-GFDL/FMS#628

@adcroft
Copy link
Collaborator

adcroft commented Dec 8, 2020

FMS 2020.04 was just released (NOAA-GFDL/FMS#632) so now we move forward on this PR.

Recall we said on a previous dev call that we'd let everyone sync up their dev branches to main and then we'd send this in isolation to main. So start preparing tags/PRs to main. Definitely a good thing to try to do during AGU, and a hackathon! 😁 At GFDL, we need to discuss which of the outstanding PRs make it into our own PR to main (#1268 might slow things down for everyone). We'll certainly not consider new ones until this is done. Looks like we should have a dev call Monday...

@marshallward marshallward merged commit f2459ec into mom-ocean:dev/gfdl Mar 26, 2021
marshallward added a commit that referenced this pull request Mar 26, 2021
+Enable FMS2 interface reads (includes merge of PR #1165)
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.

7 participants