forked from mom-ocean/MOM6
-
Notifications
You must be signed in to change notification settings - Fork 1
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 dev/gfdl as of 06 Nov 2021 #18
Merged
gustavo-marques
merged 309 commits into
ocean-eddy-cpt:dev/cpt
from
gustavo-marques:merge_dev_gfdl_06nov2021
Nov 8, 2021
Merged
(*) Merge dev/gfdl as of 06 Nov 2021 #18
gustavo-marques
merged 309 commits into
ocean-eddy-cpt:dev/cpt
from
gustavo-marques:merge_dev_gfdl_06nov2021
Nov 8, 2021
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
…L-20210723 update to GFDL 20210723 main branch
Several axes_grp pointers in register_diag_field were null()-initialized at declaration, e.g. type(axes_grp), pointer :: axes => null() This implicitly declares the pointers with `save`, which preserves its value across calls. First call excepted, these axes will not be initialized to null(). In this instance, this was causing d2 axes to be created when they should not have existed. This patch eliminates the implicit `save` attributes and applies explicit initialization when required.
Merge main from GFDL (210723)
…change_in_CFC_PR * Recover answer change from CFC PR
- Testing with an ice shelf (e.g. ISOMIP) and using the SIGMA_SHELF_ZSTAR coordinate would have caught this bug.
…solving conflicts
remove unused index bounds, and fix sum_h2 loop.
Update MOM_oda_incupd.F90
Fix to commit 893a06e which broke the SIGMA_SHELF_ZSTAR option
…0210727 IAU candidate 20210727
Add the new runtime parameter FATAL_INCONSISTENT_RESTART_TIME, which if true causes the model to compare the restart time read from a restart file with any value provided as a time_in to MOM_initialize_state and issue a fatal error if they differ. In ocean-only mode, this input time is read from a ocean_solo.res file, following FMS behavior. The default value simply uses the specified time, replicating the previous behavior. If set to true, this would prevent a problem with the time-history that recently occurred in a series of high-resolution runs that were shared between several groups, where the nominal times were repeated. All answers are bitwise identical, but many MOM_parameter_doc files have a new entry.
* +(*)Fix bug when RES_SCALE_MEKE_VISC = True Fix a bug that can lead to segmentation faults when RES_SCALE_MEKE_VISC is true, as noted in MOM6 issue mom-ocean#1464, and add better error handling to detect related problems. The refactoring that is a part of this may also avoid some problems with optimized code even when RES_SCALE_MEKE_VISC it false, as described in MOM6 issue mom-ocean#1463. In addition, logic was added so that the value of RES_SCALE_MEKE_VISC is only logged if USE_MEKE is true. All answers are bitwise identical in cases that worked, including the MOM6-examples test suite, but there are some changes in the MOM_parameter_doc files, due to an irrelevant parameter no longer being logged. * (*)Initialize CS%res_scale_MEKE Initialize the logical element res_scale_MEKE in the hor_visc_CS even if there is no Laplacian viscosity, so that a self-consistency test does not use an initialized value. Also improved some comments. All answers are bitwise identical for all cases that successfully ran before, including in all cases in the MOM6-examples test suite.
Made 37 optional arguments that are always present in calls into non-optional arguments to routines in the src/core directory. Many of these are pointer arguments related to things like open boundary conditions, so if these types are not to be used, they are simply not allocated. In several cases, this required the order of the arguments to be shifted around, but the various types of the arguments should prevent the model from compiling if the calls (e.g., in user-modified code that is not in the github repository) are not changed equivalently. Also eliminated 3 internal arguments in MOM_barotropic.F90 that are always hard-coded to the same values (the maximize argument to BT_cont_to_face_areas()) or are never used (the guess arguments to uhbt_to_ubt() and vhbt_to_vbt()), along with the code that they would exercise. All answers and output are bitwise identical.
Refactored solo_driver/MOM_driver.F90 to move logically self-contained blocks of code into separate subroutines within this file to improve the readability of the code and to reduce the number of global variables. This started out as an exercise to explore the use of the F2008 block / end block construct to reduce the scope of variables, but the code ended up being cleaner just using traditional subroutines contained in this file. All answers are bitwise identical and all output files are unaltered.
…ate-2021-10-04 Dev gfdl main candidate 2021 10 04
+Make 37 optional arguments in src/core mandatory
Made 44 previously optional just_read_params arguments into mandatory arguments and renamed them just_read to avoid duplicated variables. This change should be minimally disruptive, as these optional arguments were always being provided. Some argument documentation blocks were also reformatted or the comments in them were corrected. All answers are bitwise identical, but some arguments have changed.
Cleaned up 27 falsely optional or unused arguments in the vertical parameterization code, and related changes. This includes: - Eliminating the symmetrize arguments to set_viscous_BBL and set_viscous_ML, which are now effectively always true. - Making the Waves and OBC pointer arguments mandatory in several routines where they were always being supplied. These are pointers, so the test of whether they should be used can be based on whether they are associated. - Adding error messages about unassociated Waves types that would be used. - Eliminating the unused Waves argument to KPP_init. - Eliminating unused arguments that energetic_PBL inherited from the bulk mixed layer code, and simplified some disabled debugging code. - Making the optics argument to opacity_end mandatory. - Making the h argument to get_Langmuir_Number mandatory and rearranged the arguments to this routine to put the optional arguments last, following the practice elsewhere in the MOM6 code. Also standardized the case of several variables in the MOM_wave_interface.F90 code to facilitate searches. - Eliminating the unused US argument to CoriolisStokes. All answers are bitwise identical, and no output is changed.
This patch uses the FMS2 `file_exists` function when using the FMS2 infra. Previously, the FMS1 version of this function was being used. This patch also removes the `mpp_domain` and `no_domain` arguments from the direct `file_exist` calls, which were not used by any known MOM6 configurations. (Nor would we expect them to be, since explicit references to FMS should not exist outside of the infra layer.) Since FMS2 does not use these arguments, their removal also creates a more meaningful interface between the two frameworks. Motivation: An issue with the FMS1 `file_exists` under the FMS2 infra was discovered in the UFS model on Hera. It was only reproducible in submitted jobs, and not for interactive jobs, and only with the GCC 9.2 compiler. (Other GCC versions were not tested.) One potential explanation is that it is related to the `save` attribute of the domain pointer, `d_ptr`. In the case above, `d_ptr` pointed to the MOM input domain for the failed cases. For the other working cases, `d_ptr` pointed to a `NULL()` value and behavior was normal. It is possible that `d_ptr` is inconsisently updated when FMS1 and FMS2 IO operations are used together, which should probably be considered undefined behavior.
Eliminated the unused optional OBC argument to write_energy() and several unused optional arguments to wave_speed() and wave_speeds() that are set instead via arguments to wave_speed_init() that store these values in a wave_speed_CS type. Also made the optional row_scale argument to tridiag_det() and the tracer_CSp argument to write_energy() that were always present in calls into mandatory arguments. All answers are bitwise identical, and solutions do not change.
FMS1: Don't create IO domains for single-PE domain
Modified create_file to always specify that the threading will be for a single PE to do the writing to a single file if there is only a single ocean PE, to avoid some incorrect behavior inside of the FMS IO modules. This can fix restart problems in some single-processor cases with an inconsistent setting of the optional threading argument, but in all cases that ran correctly before, all answers are bitwise identical.
Cleaned up 13 falsely optional or unused arguments in the horizontal parameterization code, and related changes. This includes: - Made the previously optional OBC pointer arguments that were always being used in calls to 3 routines in MOM_lateral_mixing_coeffs.F90 into mandatory arguments. Because these are pointers, the deciding factor of whether to use them is really whether they are associated. - Made an internal optional argument that was always being used mandatory in 2 routines in MOM_internal_tides.F90. - Made 2 internal optional arguments that were always being used mandatory in thickness_diffuse_full(). - Eliminated the unused deta_tidal_deta argument to calc_tidal_forcing() and made the m_to_Z argument to the same routine mandatory. The former value is instead obtained by a call to tidal_sensitivity. - Eliminated 3 unused arguments and made an optional argument that was always used mandatory for find_deficit_ratios() in MOM_regularize_layers.F90.
This patch modifies the `cpu_clock_id` interface so that the `synchro_flag` argument is converted into a platform-agnostic logical flag. The current implementation requires the synchronization flag to be defined using FMS norms (zero-bit) and also would force users to follow FMS predefined flags for other values. This patch changes the sync flag to a logical, and modifies the default flag to enable synced clocks (based on `clock_flag_default`). `synchro_flag` is also renamed to `sync` for simplicity.
Use FMS2 `file_exists`, remove domain args
cpu_clock_id: synchro_flag arg changed to logical
* Eliminate GET_ALL_PARAMS in hor_visc_init Added do_not_log arguments to get_param calls in MOM_hor_visc.F90 that are only used conditionally, and eliminated the unlogged GET_ALL_PARAMS runtime parameter and get_all variable in hor_visc_init(). By design, all logging of parameters after this commit is identical to before, even for variables that are inactive and therefore should not be logged. In several places, there were some problems, mostly with the GME code, that have been noted in comments marked with '###'. Also cleaned up the code alignment and eliminated unneeded temporary variables in a few places in hor_visc(). All solutions are bitwise identical, and no output is changed. * Restore temporary variables Undid changes that eliminated temporary variables to facilitate performance profiling, and restored the "Knuth" convention about the placement of the line breaks relative to "+" in these expressions. Nothing changes.
This adds four new diagnostics building on the wind stress acceleration diagnostics du_dt_str, dv_dt_str (from mom-ocean#1437) - their thickness-weighted versions: h_du_dt_str, h_dv_dt_str (completing the set of diags from 3D thickness x momentum diagnostics mom-ocean#1398) - their viscous remnant fraction: du_dt_str_visc_rem, dv_dt_str_visc_rem (completing the set of diags from Visc_rem_[uv] multiplied momentum budget diagnostics ocean-eddy-cpt#10) Nora did some quick tests with the CPT NeverWorld2 setup, which confirm that online and offline multiplication by 1) h or 2) visc_rem_[uv] coincide (up to 1) interpolation error and 2) numerical noise, respectively), and illustrated this beautifully with some figures that accompanied the first of the three commits that were squashed into one.
Codecov Report
@@ Coverage Diff @@
## dev/cpt #18 +/- ##
===========================================
- Coverage 29.29% 29.15% -0.15%
===========================================
Files 235 239 +4
Lines 70799 71533 +734
===========================================
+ Hits 20742 20855 +113
- Misses 50057 50678 +621
Continue to review full report at Codecov.
|
gustavo-marques
pushed a commit
that referenced
this pull request
Feb 14, 2022
Bug fix for reading First_direction from restart
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 includes changes from the latest main candidate NOAA-GFDL#1507. I also includes the new wind stress acceleration diagnostics introduced in https://github.com/NOAA-GFDL/MOM6/pull/1539.
Answers for the NW2 test are identical to those obtained before #12
From July to October dev/gfdl was not correct (changing answers) and we incorrectly merged that into dev/cpt in August. The bug was in the initial topography and masking. It's a small detail in the topography that changes so no need to worry about science results - just not bitwise. This PR will bring the NW2 answers back to what they were prior to #12