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

update to MOM6 main branch 20211019 commit #78

Merged
merged 275 commits into from
Oct 28, 2021

Conversation

jiandewang
Copy link
Collaborator

@jiandewang jiandewang commented Oct 20, 2021

MOM6 main branch was updated on 20211019, need to make corresponding updating in EMC repository. This is related to MOM6 issue #77. Note this update will not change ufs answer but a new variable has been added in restart file, thus requires a new baseline.
combined with Ruiyu's ufs PR ufs-community/ufs-weather-model#863

marshallward and others added 30 commits June 16, 2021 15:03
The MOM_control_struct is declared and passed as a pointer to a CS
(usually allocated anonymously) and treated as if it were a pointer,
even though there is currently no real advantage to doing so.

After the FMS update, the deallocation of this CS was causing a
segmentation fault in the PGI compilers.  While the underlying cause was
never determined, it is likely due to some automated deallocation of the
CS contents, whose addressing became scrambled.

This problem can be resolved by moving all of the CS contents to stack,
so that the contents are automatically removed upon exiting whatever
function it was instantiated.  Subsequent calls can reference the local
(or parent) stack contents.
The MOM_control_struct is declared and passed as a pointer to a CS
(usually allocated anonymously) and treated as if it were a pointer,
even though there is currently no real advantage to doing so.

After the FMS update, the deallocation of this CS was causing a
segmentation fault in the PGI compilers.  While the underlying cause was
never determined, it is likely due to some automated deallocation of the
CS contents, whose addressing became scrambled.

This problem can be resolved by moving all of the CS contents to stack,
so that the contents are automatically removed upon exiting whatever
function it was instantiated.  Subsequent calls can reference the local
(or parent) stack contents.
This patch introduces two new testing targets to the verification suite
based on a small configuration based on the `benchmark` regression test.

The profile test is saved a `p0` in `.testing`.  Future tests can be
included if appropriate.

The new targets:

* `make profile`: Run the model and record the FMS timings.

* `make perf`: Run the model through the `perf` tool and record timings
  for the resolvable functions (as symbols).

In both cases, the timings are converted to JSON output files and the
top results are reported to stdout, and readable in GitHub actions
output.  It can also be run locally.

Support Python scripts have been included to do this work.  This will
require a functional Python environment.

Some system and configuration data is logged alongside the timings, but
this is still rather incomplete and needs some further planning.

Times are compared to the target build (usually dev/gfdl).  ANSI
terminal highlighting (i.e. color) is to used to highlight excessive
differences.

Current issues:

- Model configuration

- GitHub timings are still rather unreliable, and should currently only
  be treated as crude estimates.  This should be considered a work in
  progress.

- The GitHub profiling rule still builds the standard configuration,
  evem though it is unused.

- Additional tools are required to push the timings to some database,
  either a local sqlite3 or an external one.
- Without this change, the edges don't reproduce on restart
  due to the h values outside being nonsense.
  Added new diagnostics of the acceleration driven by the wind stress
accelerations as redistributed by a timestep of the vertical viscosity and not
lost to bottom drag within a timestep.  This is also in the diagnostics of the
accelerations due to the vertical viscosity, but the redistribution can be found
from the difference of the two.  Also added a diagnostic of the contribution of
the wind stresses to kinetic energy, and applied an underflow limiter on both
the new acceleration diagnostic and the existing viscous acceleration
diagnostic.  All solutions are bitwise identical, but there are new diagnostics.
  Add checks for inconsistent parameter settings in adiabatic_driver_init() when
ADIABATIC = True, and issue instructive error messages if any are found.  This
PR addresses MOM6 issue mom-ocean#1417.  All answers are bitwise identical, although some
cases where the inconsistent parameter settings were previously ignored may now
issue fatal errors and will not run.
  Added code to lock the restart registry once all registration should have
occurred or if the restart has been read, along with a new public interface,
restart_registry_lock, to allow this lock to be set or unset.  All calls to
register restart fields now check the state of this lock and issue a fatal error
if the registry is locked.  This PR addresses MOM6 issue mom-ocean#1214.

  In the process of adding this restart lock, the new error messages revealed
that some of the restart registration calls related to some types of open
boundary conditions were not happening early enough.  To avoid this, a new
interface, register_DOME_OBC, was added to the DOME_initialization module and is
being called from call_OBC_register, and a number of the OBC-related calls
during the initialization were collected in the same (appropriate) place.  Some
OBC error messages were also corrected.  All answers are bitwise identical, but
there are two new public interfaces and the order of some OBC-related entries in
the MOM_parameter_doc calls changed.
- This addresses the FMS issue $761
NOAA-GFDL/FMS#761

- There is a mpp_broadcast in the FMS2 subroutine
get_unlimited_dimension_name() and this subroutine has to be called by
all pes, so it cannot be inside a if(is_root_pe()) block
  Added a test to avoid attempting to deallocate the geothermal heating field if
it is not allocated, and changed the geo_heat element of geothermal_CS from a
pointer into an allocatable array.  Also clarified the comments describing
several of the elements of geothermal_CS, and added a test to avoid logging the
value of GEOTHERMAL_DRHO_DT_INPLACE when the model is not in layered-mode and
this parameter is unused.  This PR addresses MOM6 issue mom-ocean#1449.  All answers are
bitwise identical in all cases that worked before, but there are fewer entries
in some ALE-mode MOM_parameter_doc files.
marshallward and others added 20 commits September 28, 2021 14:43
Macros for generating individual rules for the tc's were added.  This
was generalized to support the contents of CONFIGS, which is now a
user-defined parameter.

The wildcard rules have now been replaced with more explicit rules, in
preparation for the merge of TESTS and TEST_TYPES.

A method for excluding tests has been added for tc3 support, and could
be extended to future tests.
The Makefile rules were extended to support multiple iterations of
dimension testing.  Examples shown below:

* test.dim      Run all dimension tests
* test.dim.l    Run all L dimension tests
* tc2.dim       Run all tc2 dimension tests
* tc2.dim.l     Run the tc2 L dimension test

Also, TESTS and TEST_TYPES were merged into a single variable, and the
old plural test names (e.g. test.grids) were removed and are now handled
as singular tests => test.grid.

The GitHub actions testing was updated to reflect these new non-plural
names.  It will take some iteration to confirm that they are working.
…direction

+(*)Add ALTERNATE_FIRST_DIRECTION
The `N2_floor` buoyancy frequency was left unset when
`KHTH_USE_FGNV_STREAMFUNCTION` was disabled.  This could potentially
cause errors, such as floating point exceptions.

Ideally we would restrict the calculations of `hN2_[uv]` to when the
streamfunction is enabled.  But due to propagation to these values to
`hN2_[xy]_PE` fields, which may be used outside of the streamfunction,
it is perhaps best to just initialize `N2_floor` to zero.

Although this would mostly likely be initialized to zero in production,
there is a chance that this could modify answers derived from random
initialization.

Thanks to @wfcooke for reporting this.  It was also independently (and
inexplicably) detected during removal of MEKE pointers, suggesting some
memory volatility.
…e_reflection

Refactoring internal tide reflection
(*) N2_floor init fix when FGNV streamfn disabled
…date-2021-10-04

correct long_name for tracer_dfy for passive tracers when diag_form == 1
PRs mom-ocean#1428 and mom-ocean#1457 extended the topography clipping to allow flooding
but missed the use case for positive-only depths where the MASKING_DEPTH
parameter alone was in use. There were two bugs:

1. The new code assumed that MINIMUM_DEPTH would be deeper than
   MASKING_DEPTH (which is intuitive). However, the point of MASKING_DEPTH
   was only to specify the determination of the land mask. The new code
   assigned depths the value of MASKING_DEPTH which broke cases that were
   using MASKING_DEPTH as documented and were leaving MINIMUM_DEPTH=0.

2. The values of variable masking_depth were altered and subsequently
   not consistent with the logged parameters. A warning was issued but
   the behavior was nevertheless not as intended.

Changes:
1. Removed the test that masking_depth > min_depth, and warning
2. Adjusted the condition and assigned value when clipping depths.
   This now uses the shallower of min_depth and masking_depth to decide
   when to clip and for the value to use otherwise.
   The expression for the land mask is unaltered.
3. Corrected documentation to retain original purpose of MASKING_DEPTH
4. Added some comments for declaration with units
5. Added some clarifying comments in code

Todo:
- resolve the need for the alternative negative depth pathway associated
  with the 0.5*min_depth expression.
- @klindsay28 spotted two issues for the NW2 tracers
  1. Use of the wrong logical variable
  2. Incorrect comment
- Using the wrong logical meant that the ideal age pacakge was being
  called in addition to the NW2 package, but did not affect the NW2
  tracers themselves.
- Following feedback from @herrwang0, we have removed the possibility
  for a user to try using negative depths without the MASKING_DEPTH
  parameter being set appropriately. This avoids the asymmetric use
  of MINIMUM_DEPTH that was proposed. A FATAL is now issued.
- Corrected a spelling error in a comment.
- Removed an unused "use" that should have been done in previous commit.
Recover topography clipping when not specifying MINIMUM_DEPTH
Correct logical associated with NW2 tracers
…ate-2021-10-04

Dev gfdl main candidate 2021 10 04
@jiandewang
Copy link
Collaborator Author

jiandewang commented Oct 25, 2021

combined with Ruiyu's ufs PR ufs-community/ufs-weather-model#863

@jiandewang
Copy link
Collaborator Author

all UFS RT done, please review

@jiandewang jiandewang merged commit 90d5961 into NOAA-EMC:dev/emc Oct 28, 2021
jiandewang pushed a commit to jiandewang/MOM6 that referenced this pull request Apr 5, 2022
+(*)Fix dimensional rescaling with HOMOGENIZE_FORCINGS
jiandewang pushed a commit to jiandewang/MOM6 that referenced this pull request May 17, 2022
Note that most of these commits are from previously squashed pull
requests, and this PR is restoring them.

- 6360dbb Merge branch 'main' into main_to_dev
- bac8031 Merge pull request mom-ocean#1566 from jiandewang/EMC-FMS-mixed-mode-20220411
- e532d86 Merge pull request NOAA-EMC#88 from marshallward/missing_attrib_with_class_bugfix
- d380f1d An alternate fix to class(*) issues with FMS 2022-01
- 8ecf333 Merge pull request NOAA-EMC#87 from jiandewang/feature/update-to-main-20220317
- ba37f94 Merge remote-tracking branch 'FSU/main' into feature/update-to-main-20220317 this is corresponding to MOM6 main 20220317 commit (hash # 399a7db)
- 44313d9 Merge pull request NOAA-EMC#85 from jiandewang/feature/update-to-main-20220217
- 966707f Merge remote-tracking branch 'GFDL/main' into feature/update-to-main-20220217 this is corresponding to MOM6 main branch 20220217 commit (hash # 6f6d4d6), which originally based on GFDL-candidate-20220129
- 32c0e1e Merge pull request NOAA-EMC#81 from jiandewang/feature/update-to-main-20211220
- 9642b1d delete external/OCEAN_stochastic_phyiscs directory as Phil re-coded in external/stochastic_physics directory
- e7c9ada solve minor conflict in mom_cap.F90 mom_ocean_model_nuopc.F90 and MOM_energetic_PBL.F90, add two new files: src/parameterizations/stochastic/MOM_stochastics.F90 and config_src/external/stochastic_physics/stochastic_physics.F90
- 90d5961 Merge pull request NOAA-EMC#78 from jiandewang/feature/update-to-GFDL-20211019
- fd02017 Merge remote-tracking branch 'GFDL/main' into feature/update-to-GFDL-20211019
- 36f17eb Merge pull request NOAA-EMC#72 from pjpegion/ocn_stoch_july2021
- a9a957e return a more accurate error message in MOM_stochasics
- 56bb41e Merge branch 'ocn_stoch_july2021' of https://github.com/pjpegion/MOM6 into ocn_stoch_july2021
- ca2ae1c update to dev/emc
- 14ca4a1 Merge pull request NOAA-EMC#76 from jiandewang/feature/update-to-GFDL-20210914
- 29016c2 Merge remote-tracking branch 'GFDL/main' into feature/update-to-GFDL-20210914 merge GFDL main 20210914 commit (hash # c09e199)
- a8577df Merge branch 'NOAA-EMC:dev/emc' into ocn_stoch_july2021
- f8a8e4c update to gfdl 20210806 (NOAA-EMC#74)
- 16e6af0 update to dev/emc
- 237a510 add comments
- 1b4273d revert logic wrt increments
- 5b2040e add logic to remove incrments from restart if outside IAU window
- c5f2b72 add write_stoch_restart_ocn to MOM_stochastics
- bdf2dc7 doxygen cleanup
- 8bc4acc move stochastics to external directory
- a3fa3a1 Merge remote-tracking branch 'upstream/dev/emc' into ocn_stoch_july2021
- e4bc007 stochastic physics re-write
- 202cbd4 update to dev/emc
- 61717ee Merge remote-tracking branch 'origin/dev/emc' into ocn_stoch
- 565e0bb remove debug statements
- a4c0411 Merge remote-tracking branch 'upstream/dev/emc' into ocn_stoch
- 689a73f remove PE_here from mom_ocean_model_nuopc.F90
- 8afe969 clean up of mom_ocean_model_nuopc.F90
- 25ed4fc revert MOM_domains.F90
- b8d9888 place stochastic array in fluxes container and make SPPT specific arrays allocatable
- d984a7e remove stochastics container
- eb88219 clean up of code for MOM6 coding standards
- 6e3ea1b correct coupled_driver/ocean_model_MOM.F90 and other cleanup
- 0b99c1f make stochastics optional
- 85023f8 Merge remote-tracking branch 'upstream/dev/emc' into ocn_stoch
- 80f9f44 clean up MOM_domains
- 5443f8e remove blank link in MOM_diagnostics
- 1727d9a re-write of stochastic code to remove CPP directives
- 600ebf9 Merge remote-tracking branch 'upstream/dev/emc' into ocn_stoch
- 6bb9d0b fix non stochastic ePBL calculation
- 1d7ffa3 clean up code
- 040e1f1 Merge pull request NOAA-EMC#13 from NOAA-EMC/dev/emc
- 2cba995 Merge branch 'dev/emc' into ocn_stoch
- 1dc0f4f Merge remote-tracking branch 'upstream/dev/emc' into dev/emc
- 4bd9b9e clean up debug statements
- 25ed5ef additions for stochy restarts
- a2a374b add stochy_restart writing to mom_cap
- 0c15f4c Update MOM_diabatic_driver.F90
- 167a62e Merge pull request #12 from pjpegion/dev/emc
- bd477a9 Update MOM_diabatic_driver.F90
- 7212400 Update MOM_diabatic_driver.F90
- 7de295c cleanup of code and enhancement of ePBL perts
- cd06356 Merge pull request #11 from NOAA-EMC/dev/emc
- 9896d61 Merge pull request #9 from pjpegion/dev/emc_merge
- 0a62737 Merge branch 'ocn_stoch' into dev/emc_merge
- 3cad1ba Merge pull request #8 from NOAA-EMC/dev/emc
- c2aa2a8 updates from dev/emc
- 182ef34 additions for stochastic physics and ePBL perts
- 671c714 Merge pull request #1 from NOAA-EMC/dev/emc
@jiandewang jiandewang deleted the feature/update-to-GFDL-20211019 branch February 28, 2023 03:54
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.