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 horiz_interp_and_extrap and set related params #286

Merged

Conversation

Hallberg-NOAA
Copy link
Member

@Hallberg-NOAA Hallberg-NOAA commented Dec 20, 2022

This series of commits revises the interfaces to horiz_interp_and_extrap to eliminate two unused arguments and rename a third from conversion to scale for consistency with other MOM6 interfaces. It also adds a set of 20 runtime parameters to replace hard coded dimensional variables, including tolerances for use with horiz_interp_and_extrap, tolerances and bounds in determine_temperature and default dynamic ice shelf temperatures. It also adds comments documenting the units of a number of internal variables in MOM_ALE_sponge. By default all answers are bitwise identical, but there are new parameters in several MOM_parameter_doc files.

The commits in this PR include:

  • 9ffa1bbc8 +Add 2 runtime params for ice shelf temperatures
  • dda6d4535 +Add 11 runtime params for determine_temperature
  • e9300da23 Document the units of variables in MOM_ALE_sponge
  • 2ed7cb194 +Add 7 runtime variables for hard-coded tolerances
  • 0e2490da4 +Revise interfaces to horiz_interp_and_extract

@codecov
Copy link

codecov bot commented Dec 20, 2022

Codecov Report

Merging #286 (5d36c08) into dev/gfdl (47fa47e) will increase coverage by 10.66%.
The diff coverage is n/a.

❗ Current head 5d36c08 differs from pull request most recent head 5573eb2. Consider uploading reports for the commit 5573eb2 to get more accurate results

@@              Coverage Diff              @@
##           dev/gfdl     #286       +/-   ##
=============================================
+ Coverage     37.07%   47.74%   +10.66%     
=============================================
  Files           263       41      -222     
  Lines         73623     4522    -69101     
  Branches      13720      792    -12928     
=============================================
- Hits          27298     2159    -25139     
+ Misses        41289     2182    -39107     
+ Partials       5036      181     -4855     
Impacted Files Coverage Δ
config_src/infra/FMS1/MOM_cpu_clock_infra.F90 0.00% <0.00%> (-78.58%) ⬇️
src/framework/MOM_string_functions.F90 28.78% <0.00%> (-58.05%) ⬇️
src/framework/MOM_coms.F90 0.00% <0.00%> (-47.72%) ⬇️
config_src/infra/FMS1/MOM_coms_infra.F90 22.60% <0.00%> (-39.73%) ⬇️
src/framework/MOM_domains.F90 0.00% <0.00%> (-37.50%) ⬇️
config_src/infra/FMS1/MOM_domain_infra.F90 0.00% <0.00%> (-36.12%) ⬇️
config_src/infra/FMS1/MOM_io_infra.F90 0.00% <0.00%> (-24.75%) ⬇️
src/framework/MOM_error_handler.F90 35.63% <0.00%> (-24.14%) ⬇️
src/framework/MOM_document.F90 63.61% <0.00%> (-11.17%) ⬇️
src/framework/MOM_file_parser.F90 89.43% <0.00%> (-1.11%) ⬇️
... and 222 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Hallberg-NOAA Hallberg-NOAA added documentation Improvements or additions to documentation Parameter change Input parameter changes (addition, removal, or description) labels Dec 23, 2022
  Revised the interface to the two horiz_interp_and_extract to eliminate the
unused reentrant_x and tripolar_n arguments and to rename the conversion
argument to scale and make it the last mandatory argument in anticipation that
we might decide to make it optional, similarly to other routines like
MOM_read_data.  The renaming is because we use 'conversion' to indicate how the
internal representation is to be rescaled for output, most prominently in
register_diag_field, whereas everywhere else where we are rescaling input to the
model's internal units, we use a scale argument.  This makes the convention
self-consistent across the MOM6 code, and should avoid some confusion.  The
calls to these routines were updated in 8 places in 4 modules.  In 8 places the
get_param calls for TRIPOLAR_N or REENTRANT_X that are no longer needed were
eliminated as were the associated internal variables.  This commit also includes
some additions to comments near the changes that were directly tied to this
commit.  All answers are bitwise identical, but there are changes to a publicly
visible interface; code that tries to use the old interface will not compile.
  Added the runtime variables DZ_BOTTOM_TOLERANCE, TRIM_IC_Z_TOLERANCE,
DENSITY_INTERP_TOLERANCE, HORIZ_INTERP_TOL_TEMP, HORIZ_INTERP_TOL_SALIN,
LAND_FILL_TEMP and LAND_FILL_SALIN to replace hard-coded dimensional constants
in the routines MOM_temp_salt_initialize_from_Z, trim_for_ice, and
initialize_thickness_from_file in MOM_state_initialization.F90.  By default, all
answers are bitwise identical, but there are new entries in the
MOM_parameter_doc files for some configurations.
  Added to the comments describing a number of the internal variables in the
MOM_ALE_sponge code, although given that much of this works on variables with
arbitrary units, many of the units descriptions have to be simply [various].
All answers and output are bitwise identical.
  Added 11 new runtime parameters (DETERMINE_TEMP_ADJUST_T_AND_S,
DETERMINE_TEMP_T_MIN, DETERMINE_TEMP_T_MAX, DETERMINE_TEMP_S_MIN,
DETERMINE_TEMP_S_MAX, DETERMINE_TEMP_T_TOLERANCE, DETERMINE_TEMP_S_TOLERANCE,
DETERMINE_TEMP_RHO_TOLERANCE, DETERMINE_TEMP_DT_DS_WEIGHT,
DETERMINE_TEMP_T_ADJ_RANGE, and DETERMINE_TEMP_S_ADJ_RANGE) to replace hard
coded dimensional parameters used in the routine determine_temperature.  This
change also requires that a param_file_type argument and the logical argument
just_read are passed to determine temperature.  By default, all answers are
bitwise identical but there are up to 10 new entries in the MOM_parameter_doc
files for some layer-mode configurations with INIT_LAYERS_FROM_Z_FILE and
FIT_TO_TARGET_DENSITY_IC set to true.
  Added the new runtime parameters INFLOW_SHELF_TEMPERATURE and
MISSING_SHELF_TEMPERATURE to the ice_shelf_dynamics module to replace hard coded
ice shelf temperatures.  White space was also added around "=" in a number of
places in this same module to align with the MOM6 style guide.  By default, all
answers are bitwise identical, but there are new entries in some
MOM_parameter_doc files for cases that use the ice shelf code with
DYNAMIC_SHELF_MASS.
@Hallberg-NOAA Hallberg-NOAA force-pushed the horiz_interp_interface branch from 9ffa1bb to 5573eb2 Compare January 2, 2023 16:16
Copy link

@MJHarrison-GFDL MJHarrison-GFDL left a comment

Choose a reason for hiding this comment

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

I agree with this collection of changes which get rid of several hard-coded values used in the tracer initialization code as well as unused arguments in horiz_interp_and_extrap_tracer routines. Based on review, these changes should not impact existing runs since all hard-coded values are previously unused runtime parameters. This should allow for dimensionally consistent re-scaling of tracers at initialization.

@@ -596,18 +598,17 @@ subroutine horiz_interp_and_extrap_tracer_record(filename, varnam, conversion,
end subroutine horiz_interp_and_extrap_tracer_record

!> Extrapolate and interpolate using a FMS time interpolation handle
subroutine horiz_interp_and_extrap_tracer_fms_id(fms_id, Time, conversion, G, tr_z, mask_z, &
z_in, z_edges_in, missing_value, reentrant_x, tripolar_n, &

Choose a reason for hiding this comment

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

These optional arguments can be safely deleted, as they are unused elsewhere. Currently reentrant_x is effectively hard-coded to True in subsequent calls to build_horiz_interp_weights. This choice should not adversely impact most applications, provided that the source grid domain is outside of the stencil of the horizontal regridding operator.

Copy link

@MJHarrison-GFDL MJHarrison-GFDL left a comment

Choose a reason for hiding this comment

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

I agree with this collection of changes which get rid of several hard-coded values used in the tracer initialization code as well as unused arguments in horiz_interp_and_extrap_tracer routines. Based on review, these changes should not impact existing runs since all hard-coded values are previously unused runtime parameters. This should allow for dimensionally consistent re-scaling of tracers at initialization.

Copy link
Member

@marshallward marshallward left a comment

Choose a reason for hiding this comment

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

Approved on behalf of @MJHarrison-GFDL

@marshallward
Copy link
Member

Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/17849 ✔️ 🟡

@marshallward marshallward merged commit e198296 into NOAA-GFDL:dev/gfdl Jan 3, 2023
@Hallberg-NOAA Hallberg-NOAA deleted the horiz_interp_interface branch February 2, 2023 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation Parameter change Input parameter changes (addition, removal, or description)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants