Skip to content

(*) Merge main as of 01 Nov 2021#16

Closed
gustavo-marques wants to merge 272 commits into
ocean-eddy-cpt:dev/cptfrom
gustavo-marques:merge_main_01Nov2021
Closed

(*) Merge main as of 01 Nov 2021#16
gustavo-marques wants to merge 272 commits into
ocean-eddy-cpt:dev/cptfrom
gustavo-marques:merge_main_01Nov2021

Conversation

@gustavo-marques
Copy link
Copy Markdown
Collaborator

Merging the main candidate described in https://github.com/NOAA-GFDL/MOM6/pull/1507

This PR changes answers for NW2.

gustavo-marques and others added 30 commits June 14, 2021 12:30
This commit adds a new module (MOM_CFC_cap) that can used to
simulate chlorofluorocarbons (CFCs) tracers. It also includes
the modifications needed to 1) allocate arrays in the fluxes and
surface types, and 2) activate the MOM_CFC_cap module via NUOPC cap.
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.
…ility

- remove the u,v,tv pointers from the set_up_oda_incupd subroutines 
- explicitly put tv,u,v in arguments of apply_oda_incupd 
- change name  get_oda_increments to calc_oda_increments 
- explicitly put tv,u,v in arguments of calc_oda_increments 
- use loops to calculate sum of h’s
- remove the 2018_answers options
- added option to output increments when using full fields 
- added subroutine output_oda_incupd_inc to write a separate double-precision file for the increments.
- add if (mask==1 ) condition to apply/calculate the increments 
-add pass_var,pass_vector on increments and u,v,tv at the end of
calc_oda_increments and apply_oda_incupd respectively
- added CS%ncount to restart file to have reproducibility on restart.
- Added option ODA_INCUPD_RESET_NCOUNT  (! default= True, If True, reinitialize number of updates already done.)
- Without this change, the edges don't reproduce on restart
  due to the h values outside being nonsense.
…mentation

* Implements a module to simulate CFCs via NUOPC cap
Fix NUOPC import of fields required for the new CFC module
The export of ice fraction was previously added for the
newly added CFC module.
- 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
Hallberg-NOAA and others added 24 commits September 24, 2021 11:18
Fix to sponge verbosity, also some documentation.
`make all` (the default rule) builds the executables specified by
`BUILDS`.  This was hard-coded, but can be promoted to a user-defined
configuration for user-defined builds.

This should be seen as a simple alias to `make build/${b}/MOM6`.

The `repro` build was also incorrectly in the list, even though it was
also conditionally added.  It has been removed from the default list.

A similar modification was made to CONFIGS, which select the "tc"
experiments.  The default is still to do a `tc*` glob.

Documentation was also added.
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
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Merging #16 (53e6e2b) into dev/cpt (7868fdd) will decrease coverage by 0.23%.
The diff coverage is 9.46%.

❗ Current head 53e6e2b differs from pull request most recent head fde9866. Consider uploading reports for the commit fde9866 to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##           dev/cpt      #16      +/-   ##
===========================================
- Coverage    29.29%   29.06%   -0.24%     
===========================================
  Files          235      237       +2     
  Lines        70799    71637     +838     
===========================================
+ Hits         20742    20822      +80     
- Misses       50057    50815     +758     
Impacted Files Coverage Δ
...g_src/drivers/solo_driver/MESO_surface_forcing.F90 0.00% <0.00%> (ø)
src/ALE/MOM_regridding.F90 21.45% <0.00%> (+0.09%) ⬆️
src/ALE/coord_adapt.F90 0.00% <0.00%> (ø)
src/ALE/regrid_interp.F90 1.41% <ø> (ø)
src/core/MOM_PressureForce_Montgomery.F90 6.36% <0.00%> (-0.02%) ⬇️
src/core/MOM_dynamics_split_RK2.F90 61.08% <0.00%> (+1.48%) ⬆️
src/core/MOM_dynamics_unsplit.F90 65.56% <0.00%> (ø)
src/core/MOM_dynamics_unsplit_RK2.F90 64.70% <0.00%> (ø)
src/core/MOM_grid.F90 61.68% <0.00%> (ø)
src/core/MOM_transcribe_grid.F90 24.63% <0.00%> (ø)
... and 120 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7868fdd...fde9866. Read the comment docs.

@gustavo-marques
Copy link
Copy Markdown
Collaborator Author

I will create a new PR from dev/gfdl to include new wind stress acceleration diagnostics from Nora.

gustavo-marques pushed a commit that referenced this pull request Feb 14, 2022
+(*)Pseudo_salt_tracer and forcing%netSalt
@gustavo-marques gustavo-marques deleted the merge_main_01Nov2021 branch February 17, 2022 15:39
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.