-
Notifications
You must be signed in to change notification settings - Fork 20
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
Merge latest mom-ocean main #254
Conversation
- Renamed function from psi(z) to mu(sigma) - Added comments and units in function mu(sigma) - Added [numerical] unit tests for mu(z), including special limits, special values, and one test value (checked against a python script).
Changes: - Allow MLE parameterization to see surface buoyancy flux return from PBL scheme (affects MOM.F90, MOM_variables.F90:vertvisc_type, MOM_diabatic_driver.F90, MOM_set_viscosity.F90) - Adds the Bodner et al., 2023, parameterization of restratification by mixed-layer eddies to MOM_mixed_layer_restrat.F90 - This is a new subroutine rather than embedded inside the previous "OM4" version. It uses different inputs, different parameters, filters the BLD differently, - Renamed mixedlayer_restrat_general to mxiedlayer_restrat_OM4 to better distinguish the two versions. - Added function rmean2ts to extend the resetting running-mean time filter used in OM4 to use different time scales when growing or decaying. While mathematically the same in the limit of a zero "growing" time-scale, the implementation differs in the use of a reciprocal instead of division so was not added to the OM4 version. - Updated module documentation Co-authored-by: Abigail Bodner <[email protected]>
This patch adds the Bodner MLE testing parameters to the tc2.a test.
Add the combined unit scaling factors Pa_to_RL2_T2 and Pa_to_RLZ_T2 to the unit_scale_type to rescale pressures and wind stresses. All answers are bitwise identical, but there are two new elements in a public type.
Use the new combined unit scaling factor US%Pa_to_RL2_T2 to rescale input pressure fields and US%Pa_to_RLZ_T2 to rescale input wind stresses in various places in the MOM6 code, including in the solo_driver and FMS_cap drivers. Analogous changes could also be made to the mct and nuopc surface forcing files, but have been omitted for now. All answers are bitwise identical.
Added the new runtime parameter TAUX_MAGNITUDE to set the strength of the zonal wind stresses when WIND_CONFIG = "2gyre", "1gyre" or "Neverworld", with a default that matches the previous hard-coded dimensional parameters that were used to specify the wind stresses in these cases. Also use US%Pa_to_RLZ_T2 to rescale wind stresses throughout solo_driver/MOM_surface_forcing.F90. By default, all answers are bitwise identical, but there is a new runtime parameter in the MOM_parameter_doc files for some test cases.
Correct inconsistent dimensional rescaling of the input values of MLD_EN_VALS, setting them all to [R Z3 T-2 ~> J m-2] to reflect that these are energies associated with vertical turbulent mixing. This fixes a rescaling bug when these energies are set to non-default values at runtime, but all answers and output are bitwise identical when no rescaling is used.
Add better error handling to read_var_sizes when a missing file or missing variable is provided as an argument. Without this change the model fails with a segmentation fault on line 768 of MOM_io.F90 if a bad file or variable name is provided. With this change, a useful error message is returned. All answers are bitwise identical in all cases that worked previously.
Redid the scaling of 52 checksum or check_redundant calls for thickness or transports to use the MKS counterparts of the thickness units (i.e., m and m3/s or kg/m2 and kg/s, depending on the Boussinesq approximation), rather than always rescaling them to m or m3/s. In Boussinesq mode, everything remains the same, but in non-Boussinesq mode, this means that the model's actual variable are being checksummed and not a version that is rescaled by division by the (meaningless?) Boussinesq reference density. All solutions are bitwise identical, but some debugging output will change in non-Boussinesq mode.
Use a conversion factor to rescale the units of masscello, just like every other diagnostic. This does not change the diagnostic itself, but it changes the order of the rescaling and the vertical remapping of this diagnostic onto other coordinates (like z) or spatial averaging of this diagnostic, which can change values in the last bits for this diagnostic for Boussinesq models (but not for non-Boussinesq models, for which the conversion factor is an integer power of 2). As a result some of the diagnostics derived from masscello can differ and this commit nominally fails the TC testing for reproducibility across code versions. All solutions and primary diagnostics, however, are bitwise identical, and even the derived diagnostic calculations are mathematically equivalent.
Remove the code to account for unit rescaling within the restart files. This rescaling within the restart files has not been used in the code since March, 2022, and the model will work with older restart files provided that they did not use dimensional rescaling, and even if they did they can be converted not to use rescaling with a short run with the older code that created them. Also removed the publicly visible routines fix_restart_scaling and eliminated the m_to_H_restart element of the verticalGrid_type; in any cases of non-standard code using this element, it should be replaced with 1.0. The various US%..._restart elements and fix_restart_unit_scaling are being retained for now because they are still being used in the SIS2 code. These changes significantly simplify the code, and they lead to a handful of constants that are always 1 not being included in the MOM6 restart files. All answers are bitwise identical, but a publicly visible interface has been eliminated, as has been an element (GV%m_to_H_restart) of a transparent type.
Added the new module MOM_EOS_Wright_full to enable the use of the version of the Wright equation of state that has been fit over the larger range of temperatures (-2 degC to 40 degC), salinities (0 psu to 40 psu) and pressures (0 dbar to 10000 dbar), than the does the restricted range fit in MOM_EOS_Wright, which had been fit over the range of (-2 degC to 30 degC), (28 psu to 38 psu) and (0 to 5000 dbar). Comments have been added to both modules to clearly document the range of properties over which they have been fitted. The new equation of state is enabled by setting EQN_OF_STATE = "WRIGHT_FULL". In addition, the default values for TFREEZE_FORM and EOS_QUADRATURE were changed depending on the equation of state to avoid having defaults that lead to fatal errors. All answers are bitwise identical in any cases that currently work, but there are new entries in the MOM_parameter_doc files. For now, only the coefficients have been changed between MOM_EOS_Wright and MOM_EOS_Wright_full, but this means that it does not yet have all of the parentheses that it should, as github.com/mom-ocean/issues/1331 discusses. A follow up PR should add appropriate self-consistency and reference value checks (with a tolerance) for the various EOS routines, and then add enough parentheses to specify the order of arithmetic and hopefully enhance the accuracy. Ideally this can be done with the new equation of state before it starts to be widely used, so that we can avoid needing a extra code to reproduce the older answers.
Cleaned up the comments describing the routines and added a proper doxygen namespace block at the end of the MOM_EOS_Wright and MOM_EOS_Wright_full modules, based on changes that A. Adcroft had on a detached branch of MOM6. Only comments are changed, and all answers are bitwise identical.
Added parentheses to all expressions with three or more additions or multiplications in the MOM_EOS_Wright_full code, so that different compilers and compiler settings will reproduce the same answers in more cases. In doing this, an effort was made to add the smallest terms first to reduce the impact of roundoff. In some cases, the code was deliberately rearranged to cancel out the leading order terms more completely. In addition, two bugs had been identified in calculate_density_second_derivs_wright_full. These were corrected and the entire routine substantially refactored with renamed variables to make the derivation easier to follow and verify. Apart from the bug corrections in the calculation of drho_dt_dt and drho_dt_dp, the changes in the expressions are mathematically equivalent, but they might make the model less noisy in some cases by reducing contributions from round-off errors. Also added comments highlighting two bugs in the drho_dt_dt and drho_dt_dp calculations in calculate_density_second_derivs_wright in the original MOM_EOS_Wright code, but did not correct them to preserve the previous answers.
Created a new module, MOM_EOS_Wright_red, that uses the reduced range fit coefficients from the Wright EOS paper, but uses the parentheses, expressions and bug fixes that are now in MOM_EOS_Wright_full. To use this new module, set EQN_OF_STATE="WRIGHT_RED". This new form is mathematically equivalent using EQN_OF_STATE="WRIGHT" (apart from correcting the bugs in the calculations of drho_dt_dt and drho_dt_dp), but the order of arithmetic is different, so the answers will differ. This change is probably as close as we can come to addressing the issues discussed at github.com/mom-ocean/issues/1331, so that issue should be closed once this commit is merged onto the main branch. Also corrected some misleading error messages in MOM_EOS and modified the code to properly handle the case for equations of state (like NEMO and UNESCO) that do not have a scalar form of calculate_density_derivs, but do have an array form. By default, all answers are bitwise identical.
Corrected a sign error in calculate_spec_vol_array_linear and calculate_spec_vol_scalar_linear when a reference specific volume is provided. This bug will cause any configurations with EQN_OF_STATE="LINEAR" and BOUSSINESQ=False (neither of which is the default value) to have the wrong sign of the pressure gradients and other serious problems, like implausible sea surface and internal interface heights. This combination of parameters would never be used in a realistic ocean model. There are no impacted cases in any of the MOM6-examples tests cases, nor those used in the ESMG or dev/NCAR test suites, and it is very unlikely that any such case would work at all. This bug was present in the original version of the calculate_spec_vol_linear routines, but it was only discovered after the implementation of the comprehensive equation of state unit testing. This will change answers in configurations that could not have worked as viable ocean models, but answers are not impacted in any known configuration, and all solutions in test cases are bitwise identical.
Added the new publicly visible function EOS_unit_tests, along with a call to it from inside of unit_tests. These tests evaluate check values for density and assess the consistency of expressions for variables that can be derived from density with finite-difference estimates of the same variables. These tests reveal inconsistencies or omissions with several of the options for the equation of state. The EOS self-consistency tests that are failing are commented out for now, so that this redacted unit test passes. All answers are bitwise identical, but there can be new diagnostic messages written out.
Changed recently added doxygen labels in the two newly added EOS_Wright_red and EOS_Wright_full modules to avoid reusing names that were already being used by EOS_Wright. All answers are bitwise identical, but the doxygen testing that had been failing for the previous 5 commits is working again.
Corrected numerous issues with the NEMO equation of state so that it is now self consistent: - Modified how coefficients are set in MOM_EOS_NEMO so that they are guaranteed to be internally self-consistent, as verified by the EOS unit tests confirming that the first derivatives of density with temperature and salinity are now consistent with the equation of state. Previously these had only been consistent to about 7 decimal places, and hence the EOS unit tests were failing for the NEMO equation of state. - Added new public interfaces to calculate_density_second_derivs_NEMO, which had previously been missing. - Added code for calculate_compress_nemo that is explicitly derived from the NEMO EOS. The previous version of calculate_compress_nemo had worked only approximately via a call to the gsw package With these changes, the NEMO EOS routines are now passing the consistency testing in the EOS unit tests. Answers will change for configurations that use the NEMO EOS to calculate any derivatives, and there are new public interfaces, but it does not appear that the NEMO equation of state is in use yet, at least it is not being used at EMC, FSU, GFDL, NASA GSFC, NCAR or in the ESMG configurations. This commit addresses the issue raised at github.com/mom-ocean/issues/405.
Added the new public interface calculate_density_second_derivs_UNESCO, which is an overload for both scalar and array versions, to calculate the second derivatives of density with various combinations of temperature, salinity and pressure. Also added a doxygen block at the end of MOM_EOS_UNESCO.F90 to describe this module and the papers it draws upon. Also replaced fatal errors in MOM_EOS with calls to these new routines. All answers are bitwise identical, but there are newly permitted combinations of options that previously failed.
Added the new public interface calc_density_second_derivs_wright_buggy to reproduce the existing answers and corrected bugs in the calculation of the second derivatives of density with temperature and with temperature and pressure in in calculate_density_second_derivs_wright. Also added the new runtime parameter USE_WRIGHT_2ND_DERIV_BUG to indicate that the older (buggy) version of calculate_density_second_derivs_wright is to be used. Most configurations will not be impacted, but by default answers will change with configurations that use the Wright equation of state and one of the Stanley or similar nonlinear EOS parameterizations, unless USE_WRIGHT_2ND_DERIV_BUG is explicitly set to True. This commit also activates the self-consistency unit testing with the Wright equation of state (now that it passes) and limited unit testing of the TEOS-10 equation of state, omitting the second derivative calculations, one of which is failing (the second derivative of density with salinity and pressure) due to a bug in the TEOS10/gsw code. Also added a unit test for consistency of the density and specific volume when an offset reference value is used.
Refactored the expressions in MOM_EOS_UNESCO.F90, adding parentheses to specify the order of arithmetic, starting with the highest-order terms first for less sensitivity to round-off. Also added comments to better describe the references for these algorithms. Although the revised expressions are all mathematically equivalent, this commit will change answers for any cases that use EQN_OF_STATE = "UNESCO". However, it is believed based on a survey of the MOM6 community that there are no active configurations that use this equation of state.
Refactored the expressions in MOM_EOS_NEMO.F90, adding parentheses to specify the order of arithmetic, starting with the highest-order terms first for less sensitivity to round-off. A number of internal variables were also renamed for greater clarity, and a number of comments were revised to better describe the references for these algorithms.. Although the revised expressions are all mathematically equivalent, this commit will change answers for any cases that use EQN_OF_STATE = "NEMO". However, there is another recent commit to this file that also changes answers (specifically the density derivatives) with this equation of state, and it is believed based on a survey of the MOM6 community that there are no active configurations that use this equation of state.
Added the new equation of state module MOM_EOS_Roquet_SpV with the polynomial specific volume fit equation of state from Roquet et al. (2015). This equation of state has also been added to MOM_EOS, where it is enabled by setting EQN_OF_STATE="ROQUET_SPV". Two other new valid settings have been added to EQN_OF_STATE, "ROQUET_RHO" and "JACKETT_MCD", which synonymous with "NEMO" and "UNESCO" respectively, but more accurately reflect the publications that describe these fits to the equation of state. The EoS unit tests are being called for the new equation of state (it passes). By default, all answers are bitwise identical, but there are numerous new publicly visible interfaces.
Added the new equation of state module MOM_EOS_Jackett06 with the rational function equation of state from Jackett et al. (2006). This uses potential temperature and practical salinity as state variables, but with a fit to more up-to-date observational data than Wright (1997) or UNESCO / Jackett and McDougall (1995). This equation of state has also been added to MOM_EOS, where it is enabled by setting EQN_OF_STATE="JACKETT_06". The EoS unit tests are being called for the new equation of state (it passes). This commit also adds slightly more output from successful EoS unit tests when run with typical levels of verbosity. By default, all answers are bitwise identical, but there are numerous new publicly visible interfaces.
Added the routine calculate_specvol_derivs_UNESCO to calculate the derivatives of specific volume with temperature and salinity to the MOM_EOS_UNESCO module. Also added some missing parentheses elsewhere in this module so that the answers will be invariant to complier version and optimization levels. Also revised the internal nomenclature of the parameters in this module to follow the conventions of the other EOS modules. Although the revised expressions are mathematically equivalent, this commit will change answers for any cases that use EQN_OF_STATE = "UNESCO". However, it is believed based on a survey of the MOM6 community that there are no active configurations that use this equation of state. There is a new publicly visible routine.
Added the new publicly visible subroutine EOS_fit_range and equivalent routines for each of the specific equation of state modules to return the range of temperatures, salinities, and pressures over which the observed data have been fitted. This is also tested for in test_EOS_consistency to indicate whether a test value is outside of the fit range, but the real purpose will be to flag and then figure out how to deal with the case when the ocean model is called with properties for which the equation of state is not valid. Note that as with all polynomial or other functional fits, extrapolating far outside of the fit range is likely to lead to bad values, but things may not be so bad for values that are only slightly outside of this range. However the question of how far out of the fit range these EoS expressions become inappropriate for each of temperature, salinity and pressure is as yet unresolved. All answers and output are bitwise identical, but there are 10 new public interfaces.
Removed unused and unnecessary #include <MOM_memory.h> statements from 5 equation of state modules. All answers are bitwise identical.
Refactored the specific volume calculations for the WRIGHT_FULL and WRIGHT_RED equations of states for simplicity or to reduce the impacts of roundoff when removing a reference value. Also added code to multiply by the reciprocal of the denominator rather than dividing in several places in the int_spec_vol_dp routines for these same two equations of state, both for efficiency and greater consistency across optimization levels. These changes are mathematically equivalent but will change answers at roundoff with these two equations of state, but they are so new that they can not have been used yet.
Renamed the module MOM_EOS_NEMO to MOM_EOS_Roquet_rho to more accurately reflect its provenance, although setting either EQN_OF_STATE = NEMO or EQN_OF_STATE = ROQUET_RHO will still work for using this code. All answers are bitwise identical, and previous input files will still work, but there are some minor changes in the MOM_parameter_doc files.
All instances of an FMS ID to the internal interpolation content is replaced with a derived type containing additional metadata recording the field's origin filename and fieldname. This additional information is required in order to replicate the axis data from the field, which is no longer provided by FMS2. The abstraction of this type also allows us to either extend it or redefine it in other frameworks as needed in the future. This primarily affects the usage of the following functions: - init_external_field - time_interp_external - horiz_interp_and_extrap_tracer The following solvers are updated: - MOM_open_boundary - MOM_ice_shelf - MOM_oda_driver - MOM_MEKE - MOM_ALE_sponge - MOM_diabatic_aux Of these, OBC was the most significant. The integer handle (fid) was previously used to determine if each segment field was constant or (if negative) read from a file. After being replaced by the derived type, a new flag was added to make this determination. All of the coupled drivers have been modified, since they support time interpolation of T and S fields. - FMS - MCT - NUOPC The NUOPC driver also includes modifications to its CFC11 and CFC12 fields. Changes to the MOM CFC modules replaces an `id == -1`-like test, which is not used by the derived type. This check has been removed, and we now solely rely on the `present(cfc_handle)` test. While this could change behavior, there does not seem to be any scenario where init_external_field would return -1 but would be passed to the function. (But I may eat these words.)
With removal of axis-based operations in FMS2 I/O, this patch removes references to these calls and replaces them with MOM `axes_info` types. References to FMS1 read into an `axistype`, but the contents are transferred to an `axis_info`. FMS2 directly populates the `axis_info` content. The `get_external_field_info` calls are modified to return `axis_info` rather than `axistype`. The redundant `get_axis_data` function is also removed from `MOM_interp_infra`, since `get_axis_info` provides an equivalent operation. Generally speaking, this is not an improvement of the codebase. The FMS1 layer does a redundant copy of data from `axistype` to `axis_info`. The FMS2 layer is significantly worse, and re-opens the file to read the axis data for each field! But if the intention is to leverage the existing API, then I don't think we have any choice at the moment. Assuming this is a relatively infrequent operation, this should not cause any measureable issues, but it needs to be watched carefully.
This patch shifts all remaining time_interp_external functions from time_interp_external to equivalent ones in time_interp_external2. Internally, time-interpolated fields are initialized with `ongrid` set to `.true.`, and such fields are assumed to be on-grid. This seems to hold for all existing instances of `time_interp_external`, but needs to be monitored in the future somehow.
The FMS1 implementation of init_external_field is case-insensitive, but the FMS2 implementation is case-sensitive, which can cause errors in older established input files. This patch sweeps through the fields of the input files and checks for a case-insensitive match (using lowercase()). This requires an additional open/close of the file.
The icebergs project now includes drivers and tests which can interfere with the coupled nolibs build, so we only pass its src directory to mkmf.
This commit includes changes to the get_param_real and log_param_real interfaces to make the units arguments mandatory. It also adds an optional unscale argument to the log_param_real interfaces. Without other changes in the previous commits, it will cause the MOM6 code to fail to compile. However, by itself this commit does not change any answers or output.
- Update actions/checkout from v2 to v3 (suggested at #231 (comment) thanks to @jedwards4b)
The FMS2 function `get_unlimited_dimension_name` raises a netCDF error if no unlimited dimension is found. This is problematic for legacy or externally created input files which may have not identifed their time axis as unlimited. This patch adds a new function, `find_unlimited_dimension_name` which mirrors the FMS2 function but returns an empty string if none are found. This is an internal function, not intended for use outside of the module.
Refactors the internal tide code in MOM_internal_tides and MOM_diabatic_driver to consolidate it in the MOM_internal_tides module and allow the control structure for that module to be made opaque. This includes moving the internal wave speed diagnostics and the call to wave_speeds or other code setting the internal wave speeds into propagate_int_tide. The get_param calls for INTERNAL_WAVE_CG1_THRESH and UNIFORM_TEST_CG were moved from the diabatic module to the MOM_internal_tides module. The wave_speed_CS and uniform_test_cg were removed from diabatic_CS and added to int_tide_CS. The Nb argument to propagate_int_tide has been made intent inout, as it is now usually set via the call to wave_speeds in that routine, but for certain tests it could use the value passed in from diabatic_driver. All answers are bitwise identical, but there are changes to public interfaces and types, and the order of some entries in the MOM_parameter_doc files and the available_diags files is changed for some cases.
Add new allocatable tau_mag arrays to the forcing and mech_forcing types to hold the magnitude of the wind stresses including gustiness contributions. There is also a new tau_mag diagnostic. This same information in tau_mag is being transformed into ustar, but these changes avoid division by the Boussinesq reference density (GV%Rho0), and allow for a more accurate calculation of derived fields when in non-Boussinesq mode, without having to multiply and divide by GV%Rho0. There is also a new optional tau_mag argument to extract_IOB_stresses to support these changes. These new arrays are not being used yet in the MOM6 solutions, but they are being allocated and populated in the routines that set the ustar fields, and they have been tested in changes to the modules that use ustar that will come in a subsequent commit. This commit also adds the new RLZ_T2_to_Pa element to the unit_scale_type to undo the scaling of wind stresses and it makes use of it in some of the new code. All answers are bitwise identical, but there are new arrays or elements in three transparent public types.
Fixed several problems with the recently added Bodner mixed layer restratification parameterization code. - Corrected the dimensional rescaling in the expressions for psi_mag by adding a missing factor of US%L_to_Z. - A logical branch was added based on the correct mask for land or OBC points to avoid potentially ill-defined calculations of the magnitude of the Bodner parameterization streamfunction, some which were leading to NaNs. - Set a tiny but nonzero default value for MIN_WSTAR2 to avoid NaNs in some calculations of the streamfunction magnitude. - Revised the expression for dd within the mu function in a mathematically equivalent way to avoid any possibility of taking a fractional exponential power of a tiny negative number due to truncation errors, which was leading to NaNs in some cases while developing and debugging the other changes that are not included in this commit. This does not appear to change any answers in the existing test cases, perhaps because the mixed layer restratification "tail" is not being activated by setting TAIL_DH to be larger than 0. - Corrected or added variable units in comments in the mixedlayer_restrat control structure. These could change answers (and avoid NaNs) in some cases with USE_BODNER23=True, MLE_TAIL_DH > 0 or MLE%TAIL_DH > 0, and there will be changes to the MOM_parameter_doc files for some cases, but given how recently this code was added, it is expected that all answers are bitwise identical in the existing test cases.
This patch adds wrappers to the set_domain and nullify_domain functions used in FMS1 for internal FMS IO operations. These are not used in FMS2, so the wrapper functions are empty. This is required to eliminate FMS1 IO dependencies in SIS2.
Correct a recently added bug in the expression for tau_mag in the nuopc_cap version of convert_IOB_to_forces, where CS%gust(i,j) was used in place of CS%gust_const, even though the 2-d array was not being set. This commit changes answers in some recent versions of the code back to what they had been previously, and it addresses concerns that had been raised with the first version of gfdl-candidate-2023-07-03 and its PR to the main version of MOM6.
* Restore functionality for reading slices from 3d volumes in MOM_io - The recent MOM_io modifications in support of FMS2_io accidentally removed support for reading on-grid data (same horizontal grid as model) k-slices. This is needed in some configurations in the model state initialization. * Add FMS1 interfaces * Additional patches to enable reading ongrid state initialization data - read local 3d volume rather than attempting to slice ongrid data vertically. - Related bugfixes in MOM_io
…07-03 GFDL candidate branch to main (2023-07-03)
GNU bug fix for MOM_variables.F90
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## dev/ncar #254 +/- ##
============================================
+ Coverage 37.07% 38.15% +1.07%
============================================
Files 264 269 +5
Lines 74421 76578 +2157
Branches 13780 14061 +281
============================================
+ Hits 27592 29215 +1623
- Misses 41739 42120 +381
- Partials 5090 5243 +153
☔ View full report in Codecov by Sentry. |
This PR brings in the latest changes from mom-ocean:main as detailed in below PRs:
mom-ocean#1603
mom-ocean#1605
This PR requires updated FMS_interface and FMS tags: fi_230818, dev/ncar_0.0.1
testing: cheyenne.aux_mom