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

Dev master candidate 2018 04 22 #912

Merged
merged 225 commits into from
Apr 25, 2019
Merged

Conversation

adcroft
Copy link
Collaborator

@adcroft adcroft commented Apr 22, 2019

This PR is to merge dev/gfdl as of 2019-04-22 onto dev/master. There are a dozen or so merges here but the big one is #896 that affects top-level APIs. There are no (known) answer changes but parameter documentation files are affected. There is a corresponding branch of MOM6-examples with updated doc files but the parameter docs for you own experiment repositories should be checked.

As per usual feedback from @gustavo-marques, @awallcraft, @kshedstrom and @jiandewang is asked for before we accept this PR.

jkrasting and others added 30 commits May 3, 2018 10:43
Merge dev/gfdl from NOAA-GFDL repository
…lations

- Changed names of module and subroutine to remove "SCM"
- Changed name of SCM ideal hurricane state initializations
- Major modifications to user/SCM_ideal_hurricane file
- To produce the full diagnostics for 1/8 degree model
  it is needed to reduce the size of output files.
  This could be done by "averaging" over a few neighboring grid cells
  and output the resulting fields on the reduced domain.
  That's what we call decimation and is the purpose of this project branch.
- Prototype zaps all diagnostics by a factor of 2
- Works only for the native grid diagnostics
- _z diagnostics complain about the local mask array index
- Next: make diag decimation optional at diag_table level
- This update allows the use to request a level 2 decimated diagnostics
  in the diag_table as following example shows

OMp5
1900 1 1 0 0 0
"ocean_hour",            0, "days",   1, "days", "time"
"ocean_model",   "tos",          "tos",              "ocean_hour",         "all", "mean", "none",2
"ocean_model",   "thetao",       "thetao",           "ocean_hour",         "all", "mean", "none",2
"ocean_model",   "umo",          "umo",              "ocean_hour",         "all", "mean", "none",2
"ocean_model",   "vmo",          "vmo",              "ocean_hour",         "all", "mean", "none",2
"ocean_model",   "volcello",     "volcello",         "ocean_hour",         "all", "mean", "none",2 # Cell measure for 3d data
"ocean_hour_d2",            0, "days",   1, "days", "time"
"ocean_model_d2",   "tos",          "tos",              "ocean_hour_d2",         "all", "mean", "none",2
"ocean_model_d2",   "thetao",       "thetao",           "ocean_hour_d2",         "all", "mean", "none",2
"ocean_model_d2",   "umo",          "umo",              "ocean_hour_d2",         "all", "mean", "none",2
"ocean_model_d2",   "vmo",          "vmo",              "ocean_hour_d2",         "all", "mean", "none",2
"ocean_model_d2",   "volcello",     "volcello",         "ocean_hour_d2",         "all", "mean", "none",2 # Cell measure for 3d data

- At the moment it works only for "Native" grid diagnostics and level 2 decimation (bination?)
- It has to be extended to non-native diagnostics, e.g.,
"ocean_model_z_d2",   "tos",          "tos",              "ocean_hour_z_d2",         "all", "mean", "none",2

- It has to be extended to arbitrary level of decimation, e.g.,
"ocean_model_z_d4",   "tos",          "tos",              "ocean_hour_z_d4",         "all", "mean", "none",2
"ocean_model_z_d2",   "tos",          "tos",              "ocean_hour_z_d2",         "all", "mean", "none",2

- Also, note that this prototype only works for smart choices of layouts where "combined" cells are on the same pe
  We need a major design revision to extend this to arbitrary layouts that would need halo updates and halo handling.
- This update allows using non-native and decimated diagnostics
  as well as their combinations. E.g., it works for a diag_table as shown below.
- I have to
      validate with a full diagnostics
      validate individual diagnostics make sense
      study the memory foot print to make sure the decimate rotuines have no leak (due to extensive use of fortran pointers)
- Also we have to work on an averaging rather than sub-sampling of the fields as is done in this prototype

OM5p5
1900 1 1 0 0 0
"ocean_hour",            0, "days",   1, "days", "time"
"ocean_model",   "tos",          "tos",              "ocean_hour",         "all", "mean", "none",2
"ocean_model",   "thetao",       "thetao",           "ocean_hour",         "all", "mean", "none",2
"ocean_model",   "umo",          "umo",              "ocean_hour",         "all", "mean", "none",2
"ocean_model",   "vmo",          "vmo",              "ocean_hour",         "all", "mean", "none",2
"ocean_model",   "volcello",     "volcello",         "ocean_hour",         "all", "mean", "none",2 # Cell measure for 3d data
"ocean_hour_d2",            0, "days",   1, "days", "time"
"ocean_model_d2",   "tos",          "tos",              "ocean_hour_d2",         "all", "mean", "none",2
"ocean_model_d2",   "thetao",       "thetao",           "ocean_hour_d2",         "all", "mean", "none",2
"ocean_model_d2",   "umo",          "umo",              "ocean_hour_d2",         "all", "mean", "none",2
"ocean_model_d2",   "vmo",          "vmo",              "ocean_hour_d2",         "all", "mean", "none",2
"ocean_model_d2",   "volcello",     "volcello",         "ocean_hour_d2",         "all", "mean", "none",2 # Cell measure for 3d data
"ocean_hour_z",            0, "days",   1, "days", "time"
"ocean_model_z",   "thetao",       "thetao",          "ocean_hour_z",         "all", "mean", "none",2
"ocean_model_z",   "umo",          "umo",             "ocean_hour_z",         "all", "mean", "none",2
"ocean_model_z",   "vmo",          "vmo",             "ocean_hour_z",         "all", "mean", "none",2
"ocean_model_z",   "volcello",     "volcello",        "ocean_hour_z",         "all", "mean", "none",2 # Cell measure for 3d data
"ocean_hour_z_d2",            0, "days",   1, "days", "time"
"ocean_model_z_d2",   "thetao",       "thetao",          "ocean_hour_z_d2",         "all", "mean", "none",2
"ocean_model_z_d2",   "umo",          "umo",             "ocean_hour_z_d2",         "all", "mean", "none",2
"ocean_model_z_d2",   "vmo",          "vmo",             "ocean_hour_z_d2",         "all", "mean", "none",2
"ocean_model_z_d2",   "volcello",     "volcello",        "ocean_hour_z_d2",         "all", "mean", "none",2 # Cell measure for 3d data
- The design of decimating subroutines with pointer manipulations was bad
  and causing memory leak. Using "allocatable" arrays instead is not
  as elegant but avoids memory leaks at the cost of bringing a few lines
  of code fo allocating temporary arrays outside the decimating subroutines.
  The FORTRAN garbage collection takes care of deallocating the "allocatable"s
  when their scope ends (unlike pointers).
- This update introduces aggregation methods, so that we can point average
  the fields rather than subsampling. This cab be extended to fancier methods
  such as area or volume averaging
- The masks for non-native decimated diags were not set right
- Some cleanup of the code to consolidate new calls
- Note that locmask => NULL() shoulbe in the body of subroutines
  not in the definition section. If it is in the definition section
  it is set to null only on the first entry (it is automatically "save"ed)
  and on subsequent entry it is whatever it was the last time.
- All decimated axes need to have the non-decimated mask3d fields initialized correctly.
  The non-decimated masks are being used in the decimation algorithm for the diagnostics fields
…ctionality in such cases.

 - wave parameters passed to split dynamics for down-Stokes gradient momentum mixing.
 - KPP updates to compute SL and turbulent Langmuir number for paramaterization options.
 - KPP bug-fixes to pass waves to correct routines for their use.
 - wave_interface bug-fix so Stokes drift bands are properly initialized.
 - The new and old (SCM) idealized_hurricane test case capabilies are included.
 - This includes a redundant subroutine SCM_idealized_hurricane_wind_forcing, which has capabilities replaced by the new idealized_hurricane_wind_forcing, but will change model answers.  When an answer change is deemed acceptable this code should be removed and a new test case should be made from the new (improved) code.
- According to Alistair, the decimation method could be solely deduced
  from the axes%x_cell_method, axes%y_cell_method and probably the area_cell_method
  at the time of send_data
- This is the summary of the algoritm
  f(Id,Jd) = \sum_{i,j} f(Id+i,Jd+j) * weight(Id+i,Jd+j) / [ \sum_{i,j} weight(Id+i,Jd+j)]
     Id,Jd are the decimated (coarse grid) indices run over the coarsened compute grid,
     i and j run from 0 to dl-1 (dl being the decimation level)
     if and jf
     weight(if,jf) run over the original fine computre grid

     x_cell_method  y_cell_method  area_cell_method    weight(if,jf)        example
--------------------------------------------------------------------- -------------
     mean           mean           mean                A(if,jf)*h(if,jf)      theta
     point          mean           mean                dy(if,jf)*h(if,jf)     u
     mean           point          mean                dx(if,jf)*h(if,jf)     v
     mean           mean           sum                 A(if,jf)               h*theta
     sum            sum            sum                 1                      volcello
     point          sum            sum                 1                      umo
Branch should be taken only if v_extensive is present AND .true., per conversation with @adcroft .
- This commit extends the proposed decimatipn algorithm to cover
  all the present diagnostics in the OM4_025 diag_table
  There may be more cases that need to be coded up later
Add missing logical condition for v_extensive
  Rescaled MIN_THICKNESS in ISOMIP initialization via the call to get_param.
Also added (commented out) unit arguments to other get_param calls.  All answers
are bitwise identical.
  Rescaled the units of Kd_add from m2 s-1 to Z2 s-1 via the get_param call for
this KD_ADD for dimensional consistency testing.  Also changed the units of the
optional Kd_int_add argument to user_change_diff and added conversion arguments
to the register_diag_field calls for Kd_user.  All answers are bitwise
identical, including rescaling Z over a large range.
  Refactored Kelvin_set_OBC_data to reduce the number of variables in the module
control structure and to use g_Earth from the vertical grid type, which required
the addition of a new verticalGrid_type argument to Kelvin_set_OBC_data.  All
answers are bitwise identical.
  Corrected a dimensional rescaling factor in the hchksum calls for Kd_int, so
the checksum output is identical when Z is rescaled.  All answers are bitwise
identical.
  Corrected a dimensional rescaling factor in the hchksum call for g_prime, so
the checksum output is identical when Z is rescaled.  All answers are bitwise
identical.
  Added code to improve the handling of underflows of shear in the kappa_shear
code, including adding parentheses to the expressions setting S2 and reading
the run-time parameter VEL_UNDERFLOW that is stored in the control structure.
Also added vel_underflow as an optional argument to calculate_projected_state.
All answers are bitwise identical, and the issues with underflows when rescaling
Z over a large range (once again at least -93 to 93) have been addressed.
Hallberg-NOAA and others added 16 commits April 9, 2019 17:26
- Missing documentation for APIs and types have been added.
- doxygen.log should now be clean of errors.
Sync dev/gfdl with NOAA-GFDL version of repository
+Added dimensional rescaling of Coriolis parameter
- Added entries to the MOM restart control structure to carry
  a list of restart variables that are no longer used
- Introduced register_restart_field_as_obsolete() subroutine
- Default behavior is to bring down the model if attempting
  to use an old restart file
- Uses register_restart_field_as_obsolete subroutine
- Cosmetic clean up of comments
- added adjustl() to string comparison - for some reason a leading
  whitespace was present with Intel compiler but not GNU.
Feature to flag and trap cases where obsolete restart fields are attempted to be used
- Uses stages to parallelize testing process
- Adds a doxygen job that tests for doxygen errors
- Reduces dependency on other repositories
  - No longer uses scripst from MOM6-examples/tools/tests/Travis-MOM6/
  - Still uses configurations from MOM6-examples/ocean_only/
- Adds test building/using a debug executable
- Adds a .testing/ directory
- Uses the latest xenial image on Travis (with openmpi)

Todo:
- Add code coverage (needs more experiments)
- Break dependence on MOM6-examples (long term project)
- Add other portable tests (e.g. parameter scaling, rotation, etc)
- Like the original cased tracer, the ca13csed tracer used in
  generic_BLING.F90 needs to be initialized to zero at all
  subsurface layers (when do_13c = .true.)
@kshedstrom
Copy link
Collaborator

kshedstrom commented Apr 23, 2019 via email

@codecov-io
Copy link

codecov-io commented Apr 23, 2019

Codecov Report

Merging #912 into dev/master will decrease coverage by <.01%.
The diff coverage is 67.77%.

Impacted file tree graph

@@              Coverage Diff               @@
##           dev/master     #912      +/-   ##
==============================================
- Coverage       53.31%   53.31%   -0.01%     
==============================================
  Files             216      217       +1     
  Lines           57043    57910     +867     
==============================================
+ Hits            30413    30872     +459     
- Misses          26630    27038     +408
Impacted Files Coverage Δ
config_src/solo_driver/MESO_surface_forcing.F90 0% <ø> (ø) ⬆️
src/tracer/MOM_tracer_registry.F90 84.37% <ø> (ø) ⬆️
...c/parameterizations/vertical/MOM_energetic_PBL.F90 74.1% <ø> (+0.21%) ⬆️
src/ALE/coord_zlike.F90 80% <ø> (ø) ⬆️
...c/parameterizations/vertical/MOM_set_viscosity.F90 70.61% <ø> (+0.06%) ⬆️
...ameterizations/lateral/MOM_mixed_layer_restrat.F90 87.16% <ø> (ø) ⬆️
...parameterizations/vertical/MOM_full_convection.F90 0% <ø> (ø) ⬆️
src/user/benchmark_initialization.F90 100% <ø> (ø) ⬆️
src/user/dumbbell_surface_forcing.F90 0% <ø> (ø) ⬆️
src/ALE/regrid_interp.F90 63.15% <ø> (ø) ⬆️
... and 242 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 e9a067f...8d92e78. Read the comment docs.

@gustavo-marques
Copy link
Collaborator

It does not change answers for us, so I approve it.

@awallcraft
Copy link
Collaborator

awallcraft commented Apr 23, 2019 via email

@jiandewang
Copy link
Collaborator

the newly introduced argument "US" in src/core/MOM_forcing_type.F90
subroutine register_forcing_type_diags(Time, diag, US, use_temperature, handles, use_berg_fluxes)
caused the EMC-NCAR unified cap failed in compiling, EMC and NCAR will need to make corresponding modification in the cap code config_src/nuopc_driver/MOM_surface_forcing.F90

  Added unit_scale_type arguments to several subroutines in the nuopc_driver
directory, added a unit_scale_type element to the nuopc_driver versoin of
ocean_state_type, and use this argument to appropriately rescale forces%ustar
for dimensional consistency testing.  These changes are required to go with the
dev-master-candidate-2018-04-22 updates to MOM6.  With these changes, the code
in origin/dev-master-candidate-2018-04-22 compiles up through the nuopc_driver
version of MOM_ocean_model.o.
@Hallberg-NOAA
Copy link
Collaborator

Sorry about the oversight with the nuopc_driver code. When we added the changes in question to the dev/gfdl code, the nuopc_driver had not yet been merged in, and of course our testing did not include the nuopc_driver when we merged it into dev/gfdl. We might need to think about how we handle testing in cases like this that might impact the various sets of top-level driver/cap code.

I just updated the nuopc_driver code to include the new missing unit_scaling_type arguments. We have now partially compiled the updated code with the nuopc_driver (as far as we can go without access to the other ESMF or nuopc code), and we believe that this should compile and run correctly with the nuopc_driver. @jiandewang, please try updating the code in this PR and see if it is working correctly for you now.

@jiandewang
Copy link
Collaborator

jiandewang commented Apr 23, 2019 via email

@jiandewang
Copy link
Collaborator

It works fine now in EMC Unified Coupling System. I approve this PR.

@gustavo-marques
Copy link
Collaborator

I just compiled the NCAR/NUOPC cap and it is working for us too.

@adcroft
Copy link
Collaborator Author

adcroft commented Apr 25, 2019

Good - I'll make the merge and then we can send out the second PR as discussed on the Monday call.

BTW. did no one notice I got the year wrong in the branch name?

@adcroft adcroft merged commit 8d92e78 into dev/master Apr 25, 2019
@adcroft adcroft deleted the dev-master-candidate-2018-04-22 branch April 25, 2019 14:11
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.