GFDL to main (2022-08-10) #1582
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Call set_initialized for variables after they are initialized, when their lack of initialization was detected via query_initialized. This commit reproduces the model's behavior prior to the removal of the code that set the initialized flag within query_initialized in #149, and it completes the set of changes that was envisioned when set_initialized was added in #152. All answers are bitwise identical.
* Add a variable to shorten some statements - suggested by @Hallberg-NOAA in the discussion for #164 * Fix to small i/j error
Where we had placed the dependency discovery (makedep) was leading to some small annoyances: - The dependency generation was being invoked twice - The dependency generation was not invoked when source code was changed. Now: - `./configure` creates config.status Makefile and Makefile.dep - `./config.status` recreates Makefile and Makefile.dep - `make depend` recreates Makefile.dep - `make` will conditionally recreate Makefile.dep Changes: - Add dependency of Makefile.dep on source code in Makefile.in. This allows `make` to know when to run `makedep`. - Fix syntax issue for AC_CONFIG_COMMANDS which caused errors to be issued without stopping. Side note: the system works perfectly well if we comment out the "AC_CONFIG_COMMANDS" line in configure.ac to invoke makedep. This is because `make` now knows how
Primarily changes to improve coverage reporting, but some other changes
as well:
* New separate rules for `codecov` and `report.cov`
* REPORT_COVERAGE was split into two flags:
* DO_COVERAGE: Enable code coverage reporting
* REQUIRE_COVERAGE_UPLOAD: If true, error if upload fails
* We now report codecov upload failures in the log, but only raise an
error (i.e. nonzero return code) if REQUIRE_COVERAGE_UPLOAD is true.
* GitHub Actions now only reports failed uploads to CodeCov for pull
requests.
* Separate FCFLAGS_FMS flag to control FCFLAGS for FMS builds
- also renamed the old FCFLAGS_FMS to FCFLAGS_DEPS, which was
previously used to pass the flags to the FMS library/modfiles
* $(DEPS) was dropped, and we just use `deps` now.
* Updated the header instructions
* README update
* Codecov.io and GitHub Actions CIs were updated to support changes
Not sure if this is wise, but codecov is severely screwing up these entries.
Updates .testing/Makefile to allow for alternative executable names and multiple executables in one build directory. Until now, the .testing/Makefile assumes all executables are called "MOM6". With the introduction of makedep, executables are called by the name indicated by the Fortran program statement. Changes: - BUILDS used to be a list of directories under build/ . Now is a list of <directory>/<exec_name>. - UNIT_EXECS lists possible executables within build/unit . This list allows us to override targets at the command-line. - Replaces many $(foreach b,$(BUILDS), ...) constructs in favor of build/%/... rules. i.e. reduces use of $(BUILDS) in general - Corrected name of executable in build/unit Co-authored-by: Marshall Ward <marshall.ward@gmail.com>
Revised the MOM_wave_structure code to avoid using array syntax in calculations and subroutine arguments. While making these changes, several incorrect or missing variable unit descriptions in comments were identified and corrected. This set of changes is mathematically equivalent, but will result in changes at the level of roundoff, either because the order of sums was changed or because divisions are replaced by multiplication by a common reciprocal. Some debugging safety checks were moved inside of a logical test of the debug flag for efficiency. One dimensionally inconsistent expression (in vertical and horizontal distances) was corrected, so that it should now pass the dimensional consistency checks. This commit will change answers in any cases that depend on this code, but because we recently corrected (in MOM6 PR#154) a major bug in this code with a much larger impact without causing any disruptions, we can be confident that this code is not yet used in an production runs where minor answer changes could be problematic. This code change does not alter any answers or output in the MOM6-examples test suite.
Corrected a sign error in commented out code in wave_structure, as noted in a review by Raf Dussin of the previous PR. All answers are bitwise identical.
DRAG_AS_BODY_FORCE was using two inconsistent values for the bottom boundary layer. It now always uses HBBL, or the total water depth if that is less than HBBL. Results with DRAG_AS_BODY_FORCE=False are unchanged.
Modified the coupler_types, data_override and time_interp modules used in the mct_cap and nuopc_cap driver code to use the appropriate modules from the MOM6 framework directory, which are properly documented and extensible, and will accommodate and buffer changes in the underlying FMS or other infrastructure code. These changes should have been in place previously, but are required to allow for rescaling of the fields being read via time_interp_external or data_override. Some unused module use references to mpp_chksum from mpp_mod were also removed. These changes mirror changes that had previously been applied to the FMS_cap driver code. All answers should be bitwise identical, but this has not been specifically tested via the GFDL testing procedures.
Applied dimensional rescaling of the ocean_heat and ocean_salt elements of the surface type. Although this surface_state is a public type, neither of these particular elements are used outside of MOM6 and these fields are not reused after they are set. They are instead being retained because they may become useful in the future. All answers are bitwise identical.
Applied temperature rescaling to the heat capacity element, C_p, of the forcing type. All answers are bitwise identical, but the rescaled units of one element of a public type were altered.
Rescaled the SST and SSS element of the surface type, usually sfc_state%SST and sfc_state%SSS, from units of [degC] and [ppt] to [degC ~> C] and [ppt ~> S], as well as a handful of other temperature and salinity variables related to the surface forcing (usually targets of restoring), and cancelled out a number of common US%C_to_degC or US%S_to_ppt conversion factors. Several unused variables were also removed, and a missing allocate and restart registration were added for the running-mean salinity in the (as yet unused) MOM_controlled_forcing module. All answers are bitwise identical, but there are changes to the rescaled units of two elements of a public type.
Use cons_temp_to_pot_temp and abs_saln_to_prac_saln to do the conversions for several diagnostics, working with rescaled variables on array segments, rather than calling gsw_pt_from_ct and gsw_sp_from_sr once from each point. All answers are bitwise identical.
Dimensionally rescaled the Stokes drift velocity variables in the mech_forcing type from [m s-1] to [L T-1 ~> m s-1], and the surface wave wavenumber variable from [rad m-1] to [rad Z-1 ~> rad m-1], eliminating several scaling factors from the code in the process, and attaching a scaling factor to a hard-coded dimensional velocity. All answers in the MOM6-examples test suite are bitwise identical.
Set default values in all get_param calls for USE_REGRIDDING. Previously, there had been 4 calls where this was missing, which led to the problems noted at mom-ocean#1576. This PR will allow that issue to be closed. Also used the default argument in a get_param call for INPUTDIR, although that case would not change any behavior because the value was set before the get_param call. A fail_in_missing argument was added to the FMS_cap call to get_param for GUST_2D_FILE, mirroring what is done for the solo_driver code, but cases where this was actually missing were very likely to have failed later anyway, but without an explicit error message. This PR could change unpredictable behavior in cases where USE_REGRIDDING is not explicitly set, but all answers are bitwise identical in the MOM6-examples test suite.
Added fail_if_missing or default arguments to 5 get_param calls in user code, with values set consistently with other calls for the same parameters. Also replaced 4 get_param calls with copies of equivalent fields from the grid type in one use initialization routine. All answers are bitwise identical.
Modified the units for 10 thickness or thickness tendency diagnostics to write them in their native units (e.g., kg m-2) when run in non-Boussinesq mode. These changes were proposed in mom-ocean#1565 and discussed and agreed to at the MOM6 dev call on 4/25/22. Previously these diagnostics had been converted to approximate thicknesses in m using a constant reference density, but this was only accurate to about 0.1% and could be misinterpreted. A number of other (more commonly used) thickness-related variables were already being written in their native units, and this makes the model's output more self consistent. All solutions are bitwise identical, but some diagnostics will change when run in non-Boussinesq mode.
Added optional answer_date arguments to various remapping routines. These are vintage-encoding integers intended to replace the logical answers_2018 arguments, and allow for multiple generations of improved algorithms rather than just two choices, without requiring added interface changes. However, this change is backward compatible, and these two arguments are both offered for now. All answers are bitwise identical, but there are new optional arguments to numerous publicly visible routines.
Replace the answers_2018 arguments with answer_date arguments to specify the version of expressions in a number of calls from the upper-level ALE modules, while also adding new answer_date optional arguments to several of the publicly visible remapping routines, including ALE_remap_scalar, regrid_set_params and remapping_set_params. The routine interpolate_grid, which is not called from outside of the regrid_interp module, is no longer being made publicly visible. Some comments noting parameters that are not guaranteed to be externally set or that can not be reset were also added. All answers are bitwise identical.
Replace answers_2018 arguments with answer_date arguments to specify the version of expressions used in a number of vertical or horizontal regridding calls. In some cases, this also involves replacing one of the elements in an opaque type. It can also involve reading (but not yet logging) the new runtime parameter DEFAULT_ANSWER_DATE, but if it not set the results are unchanged from before. There is also a new optional argument, remap_answer_date, to wave_speed_init and wave_speed_set_param. Some comments were also added to describe real variables in VarMix_init. All answers are bitwise identical.
Eliminated the now unused answers_2018 optional arguments in a variety of the ALE-related subroutines that are only called from code in the ALE directory. The functionality previously provided by answers_2018 is now provided by the more flexible answer_date arguments. A handful of spelling errors were also corrected in comments in the files that were edited. All answers are bitwise identical.
Added the new runtime parameter REMAPPING_ANSWER_DATE, which takes precedence over the older parameter REMAPPING_2018_ANSWERS. There are 11 files with get_param calls for this new parameter. Also started logging the value of DEFAULT_ANSWER_DATE. All answers are bitwise identical, but there are new entries in the MOM_parameter_doc.all files.
Added the new runtime parameter HOR_REGRID_ANSWER_DATE, which takes precedence over the older parameter HOR_REGRID_2018_ANSWERS. There are 3 files with get_param calls for this new parameter. All answers are bitwise identical, but there are new entries in the MOM_parameter_doc.all files.
Added 9 ..._ANSWER_DATE runtime parameters controlling the expressions and order of arithmetic in the parameterizations modules, which take precedence over their older ..._ANSWERS_2018 counterparts. The new runtime parameters are HOR_VISC_ANSWER_DATE, MEKE_GEOMETRIC_ANSWER_DATE, EPBL_ANSWER_DATE, OPTICS_ANSWER_DATE, REGULARIZE_LAYERS_ANSWER_DATE, SET_DIFF_ANSWER_DATE, SET_VISC_ANSWER_DATE, TIDAL_MIXING_ANSWER_DATE and VERT_FRICTION_ANSWER_DATE. All answers are bitwise identical, but there are numerous new entries in the MOM_parameter_doc.all files.
Added 6 ..._ANSWER_DATE runtime parameters controlling the expressions and order of arithmetic in the core, ocean_data_assim, user, and driver modules, which take precedence over their older ..._ANSWERS_2018 counterparts. The new runtime parameters are SURFACE_ANSWER_DATE, BAROTROPIC_ANSWER_DATE, ODA_ANSWER_DATE, IDL_HURR_ANSWER_DATE SURFACE_FORCING_ANSWER_DATE and WIND_GYRES_ANSWER_DATE. All answers are bitwise identical, but there are numerous new entries in the MOM_parameter_doc.all files.
Added do_not_log=just_read arguments to get_param calls where other calls in the same initialization routines already had them, so that the logging of the parameters is consistent. All answers are bitwise identical, but this could lead to changes in the MOM_parameter_doc files.
This patch adds the `REPORT_ERROR_LOGS` variable to the .testing and ac/deps makefiles. When set to true, a failed FMS build will also echo the contents of `config.log`, in order to diagnose remote CI builds. The variable is disabled on default, to reduce the amount of output at command line. It has been added to the GitHub Actions CI, and is propagated through `.testing/Makefile`. Although not used in the MOM6 builds, it could be extended to them if it proves useful. Currently, these Makefiles always echo `config.log` after a failed build.
This patch fixes the gcc compiler environment variables on their MacOS systems. GitHub recently made some GCC compiler changes on their MacOS machines; specifically, `gcc-11` and `gfortran-11` appear to no longer be available. Previously, we were unable to use the default gcc and had to use these executables. On this newer version, we can use the default system gcc, and the `CC` environment variable can be left unset. We still need `FC` to point to `gfortran`, since it otherwise tries to build with `f77`, which does not exist.
The implementation of inferring EKE using a neural network via a machine-learning interface has been further refactored to: - Isolate the mention of specific solutions, instead referring to a name that is more descriptive of its functionality (i.e. dbclient instead of smartredis) - The calculation of the features is also now included in the main MOM6 codebase
The changes here remove all references to specific implementations of clients used to communicate with the database. Additionally, references within MOM6 now refer "MOM_database_comms" as the module name (with similarly named methods) versus the dblcient_type. Packages are now expected to provide the following implementations which are compatible with those found in config_src/external/database_comms/MOM_database_comms.F90: - dbclient_type - dbcomms_CS_type - subroutine database_comms_init
The stub code for database_client_interface still contained references to iso_c_binding. This removes mention from it there instead requiring that various types are part of iso_fortran_env. Implementations of the methods and types by specific packages must comply with these specific types through a wrapper or directly.
Doxygen is wrongly flagging that the dummy methods in the database client stub are undocumented. This may have been related to a different change during the previous refactor of this stub. The solution for now is to simple move the docstring to precede the function
The database client stub used Fortran 2018 features which do not adhere to the MOM6 allowing only up to Fortran 2008. To fix this, separate interfaces for tensors with 1-4 dimensions replace the assumed rank subroutines. This does not interfere with the SmartRedis library as the public (overloaded) interface still retains the same API. Additionally, some methods which are likely unique to the SmartRedis client have been removed to enhance the likelihood of providing a general database client API to be used by other implementations.
GitHub appears to have restored their compiler alias conventions on their MacOS nodes. This patch restores those variables.
Codecov Report
@@ Coverage Diff @@
## main #1582 +/- ##
==========================================
+ Coverage 33.99% 37.15% +3.16%
==========================================
Files 259 261 +2
Lines 70267 71909 +1642
Branches 13023 13455 +432
==========================================
+ Hits 23885 26719 +2834
+ Misses 41880 40246 -1634
- Partials 4502 4944 +442
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
kshedstrom
approved these changes
Aug 10, 2022
marshallward
requested review from
jiandewang,
sanAkel,
abozec and
alperaltuntas
abozec
approved these changes
Aug 16, 2022
|
works fine in UFS and N. Atlantic regional setting. |
jiandewang
approved these changes
Aug 17, 2022
alperaltuntas
approved these changes
Aug 18, 2022
sanAkel
added a commit
to GEOS-ESM/GEOS_OceanGridComp
that referenced
this pull request
Aug 19, 2022
sanAkel
approved these changes
Aug 19, 2022
There was a problem hiding this comment.
Preserves answers within GEOS (@marshallward, thanks for waiting and being patient!).
|
Thanks everyone, I will merge this now. |
|
Since the If I can't figure out how to re-run these tests, I will do an admin override. |
This was referenced Aug 21, 2022
This was referenced Aug 24, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR contains two new features, some bugfixes, and an extension of the dimensional scaling to the coupler interfaces ("caps").
We now support an interface to communicate with external databases at runtime. The current proposal is very strongly designed around SmartRedis, but could be modified and configured to support other database-like systems.
Also included is a new machine learning MEKE solver which uses this database interface.
Reproducibility of legacy solvers can now be managed by an ANSWER_DATE parameter. Older flags such as ANSWERS_2018 are still supported, but can now be automatically configured by ANSWER_DATE.
Note that this patch contains changes to the driver and coupler code related to dimensional scaling, and should be reviewed carefully by anyone using them.
Also note that the git submodules to CVMix and GSW (TEOS) now point to repositories under mom-ocean, rather than the parent repositories. Source code and git hashes are equivalent, so no changes are required.
Features
Bugfix
Refactor
Meta
Contributors: