Merge branch 'NOAA-GFDL:dev/gfdl' into no-arctic-henyey

.github/actions/ubuntu-setup/action.yml

@@ -23,7 +23,7 @@ runs:
       run: |
         echo "::group::config.mk"
         cd .testing
-        echo "FCFLAGS_DEBUG = -g -O0 -Wextra -Wno-compare-reals -fbacktrace -ffpe-trap=invalid,zero,overflow -fcheck=bounds" >> config.mk
+        echo "FCFLAGS_DEBUG = -g -O0 -std=f2018 -Wextra -Wno-compare-reals -fbacktrace -ffpe-trap=invalid,zero,overflow -fcheck=bounds" >> config.mk
         echo "FCFLAGS_REPRO = -g -O2 -fbacktrace" >> config.mk
         echo "FCFLAGS_INIT = -finit-real=snan -finit-integer=2147483647 -finit-derived" >> config.mk
         echo "FCFLAGS_FMS = -g -fbacktrace -O0" >> config.mk

.github/workflows/macos-regression.yml

@@ -10,6 +10,7 @@ jobs:
     env:
       CC: gcc
       FC: gfortran
+      FMS_COMMIT: 2019.01.03
 
     defaults:
       run:

.github/workflows/macos-stencil.yml

@@ -10,6 +10,7 @@ jobs:
     env:
       CC: gcc
       FC: gfortran
+      FMS_COMMIT: 2019.01.03
 
     defaults:
       run:

.gitlab-ci.yml

@@ -32,6 +32,8 @@ p:clone:
   tags:
     - ncrc5
   script:
+    # NOTE: We could sweep any builds older than 3 days here if needed
+    #- find $HOME/ci/[0-9]* -mtime +3 -delete 2> /dev/null || true
     - .gitlab/pipeline-ci-tool.sh create-job-dir
 #.gitlab/pipeline-ci-tool.sh clean-job-dir
 
@@ -353,4 +355,5 @@ cleanup:
   before_script:
     - echo Skipping usual preamble
   script:
+    - rm -rf $HOME/ci/$CI_PIPELINE_ID
     - rm -rf $JOB_DIR

.gitlab/pipeline-ci-tool.sh

@@ -94,6 +94,13 @@ create-job-dir () {
     make -f tools/MRS/Makefile.clone clone_gfdl -j # Extras and link to datasets
     bash tools/MRS/generate_manifest.sh . tools/MRS/excluded-expts.txt > manifest.mk
     mkdir -p results
+    # Temporarily move build directory to $HOME to circumvent poor F5 performance
+    mkdir -p $HOME/ci/$CI_PIPELINE_ID/build
+    ln -s $HOME/ci/$CI_PIPELINE_ID/build build
+    # Builds need non-mangled access to src/.
+    ln -s "$(pwd)"/src $HOME/ci/$CI_PIPELINE_ID/src
+    # Static builds need access to ocean_only/
+    ln -s "$(pwd)"/ocean_only $HOME/ci/$CI_PIPELINE_ID/ocean_only
   fi
   section-end create-job-dir
 }

.testing/Makefile

@@ -20,7 +20,6 @@
 #
 # General test configuration:
 #   MPIRUN                  MPI job launcher (mpirun, srun, etc)
-#   FRAMEWORK               Model framework (fms1 or fms2)
 #   DO_REPRO_TESTS          Enable production ("repro") testing equivalence
 #   DO_REGRESSION_TESTS     Enable regression tests (usually dev/gfdl)
 #   DO_COVERAGE             Enable code coverage and generate .gcov reports
@@ -74,26 +73,31 @@ AC_SRCDIR := $(dir $(abspath $(lastword $(MAKEFILE_LIST))))../ac
 # User-defined configuration
 -include config.mk
 
-# Set the infra framework
-FRAMEWORK ?= fms2
+# Set the FMS library
+FMS_COMMIT ?= 2023.03
+FMS_URL ?= https://github.com/NOAA-GFDL/FMS.git
+export FMS_COMMIT
+export FMS_URL
 
 # Set the MPI launcher here
 # TODO: This needs more automated configuration
 MPIRUN ?= mpirun
 
-# Generic compiler variables are pass through to the builds
+# Generic compiler variables are passed through to the builds
 export CC
 export MPICC
 export FC
 export MPIFC
 
 # Builds are distinguished by FCFLAGS
-FCFLAGS_DEBUG ?= -g -O0
+FCFLAGS ?= -g -O0
+
+FCFLAGS_DEBUG ?= $(FCFLAGS)
 FCFLAGS_REPRO ?= -g -O2
 FCFLAGS_OPT ?= -g -O3 -mavx -fno-omit-frame-pointer
 FCFLAGS_INIT ?=
 FCFLAGS_COVERAGE ?= -g -O0 -fbacktrace --coverage
-FCFLAGS_FMS ?= $(FCFLAGS_DEBUG)
+FCFLAGS_FMS ?= $(FCFLAGS)
 # Additional notes:
 # - These default values are simple, minimalist flags, supported by nearly all
 #   compilers, and are somewhat analogous to GFDL's DEBUG and REPRO builds.
@@ -133,9 +137,6 @@ TIME ?= time
 WORKSPACE ?= .
 
 # Set directories for build/ and work/
-#BUILD ?= $(WORKSPACE)build
-#DEPS ?= $(BUILD)/deps
-#WORK ?= $(WORKSPACE)work
 BUILD ?= $(WORKSPACE)/build
 DEPS ?= $(BUILD)/deps
 WORK ?= $(WORKSPACE)/work
@@ -207,34 +208,6 @@ else
 endif
 
 
-# List of source files to link this Makefile's dependencies to model Makefiles
-# Assumes a depth of two, and the following extensions: F90 inc c h
-# (1): Root directory
-# NOTE: extensions could be a second variable
-SOURCE = \
-  $(foreach ext,F90 inc c h,$(wildcard $(1)/*/*.$(ext) $(1)/*/*/*.$(ext)))
-
-MOM_SOURCE = \
-  $(call SOURCE,../src) \
-  $(wildcard ../config_src/drivers/solo_driver/*.F90) \
-  $(wildcard ../config_src/ext*/*/*.F90)
-
-TARGET_SOURCE = \
-  $(call SOURCE,$(BUILD)/target_codebase/src) \
-  $(wildcard $(BUILD)/target_codebase/config_src/drivers/solo_driver/*.F90) \
-  $(wildcard $(BUILD)target_codebase/config_src/ext*/*.F90)
-
-ifeq ($(FRAMEWORK), fms1)
-  MOM_SOURCE += $(wildcard ../config_src/infra/FMS1/*.F90)
-  TARGET_SOURCE += $(wildcard $(BUILD)/target_codebase/config_src/infra/FMS1/*.F90)
-else
-  MOM_SOURCE +=$(wildcard ../config_src/infra/FMS2/*.F90)
-  TARGET_SOURCE += $(wildcard $(BUILD)/target_codebase/config_src/infra/FMS2/*.F90)
-endif
-
-FMS_SOURCE = $(call SOURCE,$(DEPS)/fms/src)
-
-
 ## Rules
 
 .PHONY: all build.regressions build.prof
@@ -286,7 +259,6 @@ $(BUILD)/unit/Makefile: MOM_ENV += $(COV_FCFLAGS) $(COV_LDFLAGS)
 $(BUILD)/timing/Makefile: MOM_ENV += $(OPT_FCFLAGS) $(MOM_LDFLAGS)
 
 # Configure script flags
-MOM_ACFLAGS := --with-framework=$(FRAMEWORK)
 $(BUILD)/openmp/Makefile: MOM_ACFLAGS += --enable-openmp
 $(BUILD)/coupled/Makefile: MOM_ACFLAGS += --with-driver=FMS_cap
 $(BUILD)/nuopc/Makefile: MOM_ACFLAGS += --with-driver=nuopc_cap
@@ -295,14 +267,26 @@ $(BUILD)/timing/Makefile: MOM_ACFLAGS += --with-driver=timing_tests
 
 
 # Build executables
+.NOTPARALLEL:$(foreach e,$(UNIT_EXECS),$(BUILD)/unit/$(e))
 $(BUILD)/unit/test_%: $(BUILD)/unit/Makefile FORCE
 	cd $(@D) && $(TIME) $(MAKE) $(@F) -j
 $(BUILD)/unit/Makefile: $(foreach e,$(UNIT_EXECS),../config_src/drivers/unit_tests/$(e).F90)
+
+.NOTPARALLEL:$(foreach e,$(TIMING_EXECS),$(BUILD)/timing/$(e))
 $(BUILD)/timing/time_%: $(BUILD)/timing/Makefile FORCE
 	cd $(@D) && $(TIME) $(MAKE) $(@F) -j
 $(BUILD)/timing/Makefile: $(foreach e,$(TIMING_EXECS),../config_src/drivers/timing_tests/$(e).F90)
+
 $(BUILD)/%/MOM6: $(BUILD)/%/Makefile FORCE
 	cd $(@D) && $(TIME) $(MAKE) $(@F) -j
+
+# Target codebase should use its own build system
+$(BUILD)/target/MOM6: $(BUILD)/target FORCE | $(TARGET_CODEBASE)
+	$(MAKE) -C $(TARGET_CODEBASE)/.testing BUILD=build build/symmetric/MOM6
+
+$(BUILD)/target: | $(TARGET_CODEBASE)
+	ln -s $(abspath $(TARGET_CODEBASE))/.testing/build/symmetric $@
+
 FORCE:
 
 
@@ -334,27 +318,12 @@ $(BUILD)/%/configure.ac: ../ac/configure.ac | $(BUILD)/%/
 $(BUILD)/%/m4/: ../ac/m4/ | $(BUILD)/%/
 	cp -r ../ac/m4 $(@D)
 
-ALL_EXECS = symmetric asymmetric repro openmp target opt opt_target coupled \
-  nuopc cov unit timing
+ALL_EXECS = symmetric asymmetric repro openmp opt opt_target coupled nuopc \
+  cov unit timing
 $(foreach b,$(ALL_EXECS),$(BUILD)/$(b)/):
 	mkdir -p $@
 
 # Fetch the regression target codebase
-
-$(BUILD)/target/config.status: $(BUILD)/target/configure $(DEPS)/lib/libFMS.a
-	cd $(@D) && $(MOM_ENV) ./configure -n \
-	  --srcdir=$(abspath $(BUILD))/target_codebase/ac $(MOM_ACFLAGS) \
-	|| (cat config.log && false)
-
-$(BUILD)/target/Makefile.in: | $(TARGET_CODEBASE) $(BUILD)/target/
-	cp $(TARGET_CODEBASE)/ac/Makefile.in $(@D)
-
-$(BUILD)/target/configure.ac: | $(TARGET_CODEBASE) $(BUILD)/target/
-	cp $(TARGET_CODEBASE)/ac/configure.ac $(@D)
-
-$(BUILD)/target/m4/: | $(TARGET_CODEBASE) $(BUILD)/target/
-	cp -r $(TARGET_CODEBASE)/ac/m4 $(@D)
-
 $(TARGET_CODEBASE):
 	git clone --recursive $(MOM_TARGET_URL) $@
 	cd $@ && git checkout --recurse-submodules $(MOM_TARGET_BRANCH)
@@ -684,7 +653,7 @@ $(WORK)/%/restart/ocean.stats: $(BUILD)/symmetric/MOM6 | preproc
 # Not a true rule; only call this after `make test` to summarize test results.
 .PHONY: test.summary
 test.summary:
-	@./tools/report_test_results.sh $(WORK)/results
+	./tools/report_test_results.sh $(WORK)/results
 
 
 #---

.testing/README.rst

@@ -47,9 +47,11 @@ Several of the following may require configuration for particular systems.
    Name of the MPI launcher.  Often this is ``mpirun`` or ``mpiexec`` but may
    all need to run through a scheduler, e.g. ``srun`` if using Slurm.
 
-``FRAMEWORK`` (*default:* ``fms1``)
-   Select either the legacy FMS framework (``fms1``) or an FMS2 I/O compatible
-   version (``fms2``).
+``FMS_COMMIT`` (*default:* ``2023.03``)
+   Set the FMS version, either by tag or commit (as defined in ``FMS_URL``).
+
+``FMS_URL`` (*default*: ``https://github.com/NOAA-GFDL/FMS.git``)
+   Set the URL of the FMS repository.
 
 ``DO_REPRO_TESTS`` (*default:* *none*)
    Set to ``true`` to test the REPRO build and confirm equivalence of DEBUG and

.testing/tc1/MOM_input

@@ -278,6 +278,12 @@ BOUND_CORIOLIS = True           !   [Boolean] default = False
                                 ! have no effect on the SADOURNY Coriolis scheme if it
                                 ! were possible to use centered difference thickness fluxes.
 
+! === module MOM_PressureForce_FV ===
+MASS_WEIGHT_IN_PRESSURE_GRADIENT = True !   [Boolean] default = False
+                                ! If true, use mass weighting when interpolating T/S for integrals
+                                ! near the bathymetry in FV pressure gradient calculations.
+
+
 ! === module MOM_hor_visc ===
 AH_VEL_SCALE = 0.05             !   [m s-1] default = 0.0
                                 ! The velocity scale which is multiplied by the cube of

.testing/tc2/MOM_input

@@ -302,11 +302,16 @@ PGF_STANLEY_T2_DET_COEFF = -1.0 !   [nondim] default = -1.0
                                 ! gradient in the deterministic part of the Stanley form of the Brankart
                                 ! correction. Negative values disable the scheme.
 
+! === module MOM_PressureForce_FV ===
+MASS_WEIGHT_IN_PRESSURE_GRADIENT = True !   [Boolean] default = False
+                                ! If true, use mass weighting when interpolating T/S for integrals
+                                ! near the bathymetry in FV pressure gradient calculations.
+
 ! === module MOM_hor_visc ===
 LAPLACIAN = True
 KH_VEL_SCALE = 0.05
 SMAGORINSKY_KH = True           !   [Boolean] default = False
-SMAG_LAP_CONST = 0.06            !   [nondim] default = 0.0
+SMAG_LAP_CONST = 0.06           !   [nondim] default = 0.0
 AH_VEL_SCALE = 0.05             !   [m s-1] default = 0.0
                                 ! The velocity scale which is multiplied by the cube of
                                 ! the grid spacing to calculate the Laplacian viscosity.

.testing/tc4/MOM_input

@@ -269,6 +269,9 @@ BOUND_CORIOLIS = True           !   [Boolean] default = False
 ! === module MOM_PressureForce ===
 
 ! === module MOM_PressureForce_FV ===
+MASS_WEIGHT_IN_PRESSURE_GRADIENT = True !   [Boolean] default = False
+                                ! If true, use mass weighting when interpolating T/S for integrals
+                                ! near the bathymetry in FV pressure gradient calculations.
 RECONSTRUCT_FOR_PRESSURE = False !   [Boolean] default = True
                                 ! If True, use vertical reconstruction of T & S within the integrals of the FV
                                 ! pressure gradient calculation. If False, use the constant-by-layer algorithm.

.testing/tools/parse_perf.py

@@ -3,10 +3,20 @@
 import collections
 import json
 import os
+import re
 import shlex
 import subprocess
 import sys
 
+perf_scanner = re.Scanner([
+  (r'<', lambda scanner, token: token),
+  (r'>', lambda scanner, token: token),
+  (r'\(', lambda scanner, token: token),
+  (r'\)', lambda scanner, token: token),
+  (r'[ \t]+', lambda scanner, token: token),
+  (r'[^<>() \t]+', lambda scanner, token: token),
+])
+
 
 def main():
     desc = 'Parse perf.data and return in JSON format.'
@@ -58,15 +68,55 @@ def parse_perf_report(perf_data_path):
 
             # get per-symbol count
             else:
+                tokens, remainder = perf_scanner.scan(line)
+                if remainder:
+                    print('Line could not be tokenized', file=sys.stderr)
+                    print(' line:', repr(line), file=sys.stderr)
+                    print(' tokens:', tokens, file=sys.stderr)
+                    print(' remainder:', remainder, file=sys.stderr)
+                    sys.exit(os.EX_DATAERR)
+
+                # Construct record from tokens
+                # (NOTE: Not a proper grammar, just dumb bracket counting)
+                record = []
+                bracks = 0
+                parens = 0
+
+                for tok in tokens:
+                    if tok == '<':
+                        bracks += 1
+
+                    if tok == '(':
+                        parens += 1
+
+                    rec = record[-1] if record else None
+
+                    inside_bracket = rec and (bracks > 0 or parens > 0)
+                    lead_rec = tok in '<(' and rec and not rec.isspace()
+                    tail_rec = not tok.isspace() and rec and rec[-1] in '>)'
+
+                    if inside_bracket or lead_rec or tail_rec:
+                        record[-1] += tok
+                    else:
+                        record.append(tok)
+
+                    if tok == '>':
+                        bracks -= 1
+                    if tok == ')':
+                        parens -= 1
+
+                # Strip any whitespace tokens
+                record = [rec for rec in record if not rec.isspace()]
+
                 try:
-                    tokens = line.split()
-                    symbol = tokens[2]
-                    period = int(tokens[3])
-                except ValueError:
+                    symbol = record[2]
+                    period = int(record[3])
+                except:
                     print("parse_perf.py: Error extracting symbol count",
-                            file=sys.stderr)
+                          file=sys.stderr)
                     print("line:", repr(line), file=sys.stderr)
                     print("tokens:", tokens, file=sys.stderr)
+                    print("record:", record, file=sys.stderr)
                     raise
 
                 profile[event_name]['symbol'][symbol] = period