Skip to content

Commit

Permalink
Merge pull request #119 from marshallward/unittest
Browse files Browse the repository at this point in the history
MOM_file_parser unit test implementation
  • Loading branch information
Hallberg-NOAA authored May 20, 2022
2 parents 9d6def6 + cf448a1 commit 9292b58
Show file tree
Hide file tree
Showing 11 changed files with 2,411 additions and 32 deletions.
5 changes: 3 additions & 2 deletions .codecov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ coverage:
threshold: 100%
base: parent
comment:
# This must be set to the number of test cases (TCs)
after_n_builds: 8
# This is set to the number of TCs, plus unit, but can be removed
# (i.e. set to 1) when reporting is separated from coverage.
after_n_builds: 9
1 change: 0 additions & 1 deletion .github/actions/testing-setup/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ runs:
echo "FCFLAGS_DEBUG=-g -O0 -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_COVERAGE=--coverage" >> config.mk
cat config.mk
echo "::endgroup::"
Expand Down
11 changes: 10 additions & 1 deletion .github/workflows/coverage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,14 @@ jobs:

- uses: ./.github/actions/testing-setup

- name: Compile unit testing
run: make -j build/unit/MOM6

- name: Run unit tests
run: make unit.cov.upload

- name: Compile MOM6 with code coverage
run: make -j build/cov/MOM6

- name: Run and post coverage
run: make run.symmetric -k -s
run: make run.cov -k -s
105 changes: 80 additions & 25 deletions .testing/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
# MPIRUN MPI job launcher (mpirun, srun, etc)
# DO_REPRO_TESTS Enable production ("repro") testing equivalence
# DO_REGRESSION_TESTS Enable regression tests (usually dev/gfdl)
# REPORT_COVERAGE Enable code coverage and report to codecov
# REPORT_COVERAGE Enable code coverage and generate reports
#
# Compiler configuration:
# CC C compiler
Expand Down Expand Up @@ -82,11 +82,11 @@ export MPIFC
FCFLAGS_DEBUG ?= -g -O0
FCFLAGS_REPRO ?= -g -O2
FCFLAGS_OPT ?= -g -O3 -mavx -fno-omit-frame-pointer
FCFLAGS_COVERAGE ?= -g -O0 -fbacktrace --coverage
FCFLAGS_INIT ?=
FCFLAGS_COVERAGE ?=
# Additional notes:
# - These default values are simple, minimalist flags, supported by nearly all
# compilers which are comparable to GFDL's canonical DEBUG and REPRO builds.
# compilers, and are comparable to GFDL's canonical DEBUG and REPRO builds.
#
# - These flags should be configured outside of the Makefile, either with
# config.mk or as environment variables.
Expand All @@ -95,6 +95,7 @@ FCFLAGS_COVERAGE ?=
# so FCFLAGS_INIT is used to provide additional MOM6 configuration.

# User-defined LDFLAGS (applied to all builds and FMS)
LDFLAGS_COVERAGE ?= --coverage
LDFLAGS_USER ?=

# Set to `true` to require identical results from DEBUG and REPRO builds
Expand Down Expand Up @@ -139,6 +140,9 @@ ifeq ($(DO_PROFILE), false)
BUILDS += opt opt_target
endif

# Unit test testing
BUILDS += cov unit

# The following variables are configured by Travis:
# DO_REGRESSION_TESTS: true if $(TRAVIS_PULL_REQUEST) is a PR number
# MOM_TARGET_SLUG: TRAVIS_REPO_SLUG
Expand All @@ -165,8 +169,6 @@ else
TARGET_CODEBASE =
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
Expand Down Expand Up @@ -220,10 +222,8 @@ build.prof: $(foreach b,opt opt_target,build/$(b)/MOM6)
BUILD_TARGETS = MOM6 Makefile path_names
.PRECIOUS: $(foreach b,$(BUILDS),$(foreach f,$(BUILD_TARGETS),build/$(b)/$(f)))

# Compiler flags

# Conditionally build symmetric with coverage support
COVERAGE=$(if $(REPORT_COVERAGE),$(FCFLAGS_COVERAGE),)
# Compiler flags

# .testing dependencies
# TODO: We should probably build TARGET with the FMS that it was configured
Expand All @@ -234,28 +234,31 @@ PATH_FMS = PATH="${PATH}:../../$(DEPS)/bin"


# Define the build targets in terms of the traditional DEBUG/REPRO/etc labels
SYMMETRIC_FCFLAGS := FCFLAGS="$(FCFLAGS_DEBUG) $(FCFLAGS_INIT) $(COVERAGE) $(FCFLAGS_FMS)"
SYMMETRIC_FCFLAGS := FCFLAGS="$(FCFLAGS_DEBUG) $(FCFLAGS_INIT) $(FCFLAGS_FMS)"
ASYMMETRIC_FCFLAGS := FCFLAGS="$(FCFLAGS_DEBUG) $(FCFLAGS_INIT) $(FCFLAGS_FMS)"
REPRO_FCFLAGS := FCFLAGS="$(FCFLAGS_REPRO) $(FCFLAGS_FMS)"
OPT_FCFLAGS := FCFLAGS="$(FCFLAGS_OPT) $(FCFLAGS_FMS)"
OPENMP_FCFLAGS := FCFLAGS="$(FCFLAGS_DEBUG) $(FCFLAGS_INIT) $(FCFLAGS_FMS)"
TARGET_FCFLAGS := FCFLAGS="$(FCFLAGS_DEBUG) $(FCFLAGS_INIT) $(FCFLAGS_FMS)"
COV_FCFLAGS := FCFLAGS="$(FCFLAGS_COVERAGE) $(FCFLAGS_FMS)"

MOM_LDFLAGS := LDFLAGS="$(LDFLAGS_FMS) $(LDFLAGS_USER)"
SYMMETRIC_LDFLAGS := LDFLAGS="$(COVERAGE) $(LDFLAGS_FMS) $(LDFLAGS_USER)"
COV_LDFLAGS := LDFLAGS="$(LDFLAGS_COVERAGE) $(LDFLAGS_FMS) $(LDFLAGS_USER)"


# Environment variable configuration
build/symmetric/Makefile: MOM_ENV=$(PATH_FMS) $(SYMMETRIC_FCFLAGS) $(SYMMETRIC_LDFLAGS)
build/symmetric/Makefile: MOM_ENV=$(PATH_FMS) $(SYMMETRIC_FCFLAGS) $(MOM_LDFLAGS)
build/asymmetric/Makefile: MOM_ENV=$(PATH_FMS) $(ASYMMETRIC_FCFLAGS) $(MOM_LDFLAGS)
build/repro/Makefile: MOM_ENV=$(PATH_FMS) $(REPRO_FCFLAGS) $(MOM_LDFLAGS)
build/openmp/Makefile: MOM_ENV=$(PATH_FMS) $(OPENMP_FCFLAGS) $(MOM_LDFLAGS)
build/target/Makefile: MOM_ENV=$(PATH_FMS) $(TARGET_FCFLAGS) $(MOM_LDFLAGS)
build/opt/Makefile: MOM_ENV=$(PATH_FMS) $(OPT_FCFLAGS) $(MOM_LDFLAGS)
build/opt_target/Makefile: MOM_ENV=$(PATH_FMS) $(OPT_FCFLAGS) $(MOM_LDFLAGS)
build/coupled/Makefile: MOM_ENV=$(PATH_FMS) $(SYMMETRIC_FCFLAGS) $(SYMMETRIC_LDFLAGS)
build/nuopc/Makefile: MOM_ENV=$(PATH_FMS) $(SYMMETRIC_FCFLAGS) $(SYMMETRIC_LDFLAGS)
build/mct/Makefile: MOM_ENV=$(PATH_FMS) $(SYMMETRIC_FCFLAGS) $(SYMMETRIC_LDFLAGS)
build/coupled/Makefile: MOM_ENV=$(PATH_FMS) $(SYMMETRIC_FCFLAGS) $(MOM_LDFLAGS)
build/nuopc/Makefile: MOM_ENV=$(PATH_FMS) $(SYMMETRIC_FCFLAGS) $(MOM_LDFLAGS)
build/mct/Makefile: MOM_ENV=$(PATH_FMS) $(SYMMETRIC_FCFLAGS) $(MOM_LDFLAGS)
build/cov/Makefile: MOM_ENV=$(PATH_FMS) $(COV_FCFLAGS) $(COV_LDFLAGS)
build/unit/Makefile: MOM_ENV=$(PATH_FMS) $(COV_FCFLAGS) $(COV_LDFLAGS)

# Configure script flags
build/symmetric/Makefile: MOM_ACFLAGS=
Expand All @@ -268,6 +271,8 @@ build/opt_target/Makefile: MOM_ACFLAGS=
build/coupled/Makefile: MOM_ACFLAGS=--with-driver=FMS_cap
build/nuopc/Makefile: MOM_ACFLAGS=--with-driver=nuopc_cap
build/mct/Makefile: MOM_ACFLAGS=--with-driver=mct_cap
build/cov/Makefile: MOM_ACFLAGS=
build/unit/Makefile: MOM_ACFLAGS=--with-driver=unit_tests

# Fetch regression target source code
build/target/Makefile: | $(TARGET_CODEBASE)
Expand All @@ -276,7 +281,7 @@ build/opt_target/Makefile: | $(TARGET_CODEBASE)

# Define source code dependencies
# NOTE: ./configure is too much, but Makefile is not enough!
# Ideally we would want to re-run both Makefile and mkmf, but our mkmf call
# Ideally we only want to re-run both Makefile and mkmf, but the mkmf call
# is inside ./configure, so we must re-run ./configure as well.
$(foreach b,$(filter-out target,$(BUILDS)),build/$(b)/Makefile): $(MOM_SOURCE)
build/target_codebase/configure: $(TARGET_SOURCE)
Expand Down Expand Up @@ -362,11 +367,13 @@ $(DEPS)/Makefile: ../ac/deps/Makefile


#---
# The following block does a non-library build of a coupled driver interface to MOM, along with everything below it.
# This simply checks that we have not broken the ability to compile. This is not a means to build a complete coupled executable.
# Todo:
# - avoid re-building FMS and MOM6 src by re-using existing object/mod files
# - use autoconf rather than mkmf templates
# The following block does a non-library build of a coupled driver interface to
# MOM, along with everything below it. This simply checks that we have not
# broken the ability to compile. This is not a means to build a complete
# coupled executable.
# TODO:
# - Avoid re-building FMS and MOM6 src by re-using existing object/mod files
# - Use autoconf rather than mkmf templates
MK_TEMPLATE ?= ../../$(DEPS)/mkmf/templates/ncrc-gnu.mk
# NUOPC driver
build/nuopc/mom_ocean_model_nuopc.o: build/nuopc/Makefile
Expand Down Expand Up @@ -425,11 +432,12 @@ test.dim.$(1): $(foreach c,$(CONFIGS),$(c).dim.$(1) $(c).dim.$(1).diag)
endef
$(foreach d,$(DIMS),$(eval $(call TEST_DIM_RULE,$(d))))

.PHONY: run.symmetric run.asymmetric run.nans run.openmp
.PHONY: run.symmetric run.asymmetric run.nans run.openmp run.cov
run.symmetric: $(foreach c,$(CONFIGS),work/$(c)/symmetric/ocean.stats)
run.asymmetric: $(foreach c,$(filter-out tc3,$(CONFIGS)),$(CONFIGS),work/$(c)/asymmetric/ocean.stats)
run.nan: $(foreach c,$(CONFIGS),work/$(c)/nan/ocean.stats)
run.openmp: $(foreach c,$(CONFIGS),work/$(c)/openmp/ocean.stats)
run.cov: $(foreach c,$(CONFIGS),work/$(c)/cov/ocean.stats)

# Configuration test rules
# $(1): Configuration name (tc1, tc2, &c.)
Expand Down Expand Up @@ -573,11 +581,11 @@ work/%/$(1)/ocean.stats work/%/$(1)/chksum_diag: build/$(2)/MOM6 $(VENV_PATH)
@echo -e "$(DONE): $$*.$(1); no runtime errors."
if [ $(3) ]; then \
mkdir -p results/$$* ; \
cd build/symmetric ; \
gcov *.gcda > gcov.$$*.$(1).out ; \
cd build/$(2) ; \
gcov -b *.gcda > gcov.$$*.$(1).out ; \
curl -s $(CODECOV_UPLOADER_URL) -o codecov ; \
chmod +x codecov ; \
./codecov -Z -f "*.gcov" -n $$@ \
./codecov -R . -Z -f "*.gcov" -n $$@ \
> codecov.$$*.$(1).out \
2> codecov.$$*.$(1).err \
&& echo -e "${MAGENTA}Report uploaded to codecov.${RESET}"; \
Expand All @@ -603,6 +611,7 @@ $(eval $(call STAT_RULE,dim.z,symmetric,,Z_RESCALE_POWER=11,,1))
$(eval $(call STAT_RULE,dim.q,symmetric,,Q_RESCALE_POWER=11,,1))
$(eval $(call STAT_RULE,dim.r,symmetric,,R_RESCALE_POWER=11,,1))

$(eval $(call STAT_RULE,cov,cov,$(REPORT_COVERAGE),,,1))

# Generate the half-period input namelist as follows:
# 1. Fetch DAYMAX and TIMEUNIT from MOM_input
Expand Down Expand Up @@ -652,7 +661,6 @@ work/%/restart/ocean.stats: build/symmetric/MOM6 $(VENV_PATH)

# TODO: Restart checksum diagnostics


#---
# Not a true rule; only call this after `make test` to summarize test results.
.PHONY: test.summary
Expand All @@ -679,6 +687,53 @@ test.summary:
fi


#---
# unit test

.PHONY: unit.cov
unit.cov: build/unit/MOM_new_unit_tests.gcov

work/unit/std.out: build/unit/MOM6
if [ $(REPORT_COVERAGE) ]; then \
find build/unit -name *.gcda -exec rm -f '{}' \; ; \
fi
rm -rf $(@D)
mkdir -p $(@D)
cd $(@D) \
&& $(TIME) $(MPIRUN) -n 1 ../../$< 2> std.err > std.out \
|| !( \
cat std.out | tail -n 100 ; \
cat std.err | tail -n 100 ; \
)
cd $(@D) \
&& $(TIME) $(MPIRUN) -n 2 ../../$< 2> p2.std.err > p2.std.out \
|| !( \
cat p2.std.out | tail -n 100 ; \
cat p2.std.err | tail -n 100 ; \
)

build/unit/codecov:
mkdir -p $(@D)
cd $(@D) \
&& curl -s $(CODECOV_UPLOADER_URL) -o $(@F)
chmod +x $@

# Use driver coverage file as a proxy for the run
# TODO: Replace work/unit/std.out with *.gcda?
build/unit/MOM_new_unit_tests.gcov: work/unit/std.out
mkdir -p $(@D)
cd $(@D) \
&& gcov -b *.gcda > gcov.unit.out

# Use driver coverage file as a proxy for the run
.PHONY: unit.cov.upload
unit.cov.upload: build/unit/MOM_new_unit_tests.gcov build/unit/codecov
cd build/unit \
&& ./codecov -R . -Z -f "*.gcov" -n "Unit tests" \
> codecov.unit.out \
2> codecov.unit.err \
&& echo -e "${MAGENTA}Report uploaded to codecov.${RESET}"

#---
# Profiling
# XXX: This is experimental work to track, log, and report changes in runtime
Expand Down
6 changes: 5 additions & 1 deletion ac/configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,11 @@ AS_IF([test "$enable_asymmetric" = yes],
# Default to solo_driver
DRIVER_DIR=${srcdir}/config_src/drivers/solo_driver
AC_ARG_WITH([driver],
AS_HELP_STRING([--with-driver=coupled_driver|solo_driver], [Select directory for driver source code]))
AS_HELP_STRING(
[--with-driver=coupled_driver|solo_driver|unit_tests],
[Select directory for driver source code]
)
)
AS_IF([test "x$with_driver" != "x"],
[DRIVER_DIR=${srcdir}/config_src/drivers/${with_driver}])

Expand Down
65 changes: 65 additions & 0 deletions config_src/drivers/unit_tests/MOM_unit_test_driver.F90
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
program MOM_unit_tests

use MPI
use MOM_domains, only : MOM_infra_init
use MOM_domains, only : MOM_infra_end
use MOM_file_parser_tests, only : run_file_parser_tests

implicit none

integer, parameter :: comm = MPI_COMM_WORLD
integer, parameter :: root = 0
integer :: rank
logical :: file_exists_on_rank
logical :: input_nml_exists, MOM_input_exists
integer :: io_unit
logical :: is_open, is_file
integer :: rc

! NOTE: Bootstrapping requires external MPI configuration.
! - FMS initialization requires the presence of input.nml
! - MOM initialization requires MOM_input (if unspecificed by input.nml)
! - Any MPI-based I/O prior to MOM and FMS init will MPI initialization
! Thus, we need to do some minimal MPI setup.
call MPI_Init(rc)
call MPI_Comm_rank(comm, rank, rc)

inquire(file='input.nml', exist=file_exists_on_rank)
call MPI_Reduce(file_exists_on_rank, input_nml_exists, 1, MPI_LOGICAL, &
MPI_LOR, root, comm, rc)

inquire(file='MOM_input', exist=file_exists_on_rank)
call MPI_Reduce(file_exists_on_rank, MOM_input_exists, 1, MPI_LOGICAL, &
MPI_LOR, root, comm, rc)

if (rank == root) then
! Abort if at least one rank sees either input.nml or MOM_input
if (input_nml_exists) error stop "Remove existing 'input.nml' file."
if (MOM_input_exists) error stop "Remove existing 'MOM_input' file."

! Otherwise, create the (empty) files
open(newunit=io_unit, file='input.nml', status='replace')
write(io_unit, '(a)') "&fms2_io_nml /"
close(io_unit)

open(newunit=io_unit, file='MOM_input', status='replace')
close(io_unit)
endif

call MOM_infra_init(comm)

! Run tests
call run_file_parser_tests

! Cleanup
call MOM_infra_end

if (rank == root) then
open(newunit=io_unit, file='MOM_input')
close(io_unit, status='delete')

open(newunit=io_unit, file='input.nml')
close(io_unit, status='delete')
endif

end program MOM_unit_tests
9 changes: 8 additions & 1 deletion config_src/infra/FMS1/MOM_coms_infra.F90
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ module MOM_coms_infra

implicit none ; private

public :: PE_here, root_PE, num_PEs, set_rootPE, Set_PElist, Get_PElist
public :: PE_here, root_PE, num_PEs, set_rootPE, Set_PElist, Get_PElist, sync_PEs
public :: broadcast, sum_across_PEs, min_across_PEs, max_across_PEs
public :: any_across_PEs, all_across_PEs
public :: field_chksum, MOM_infra_init, MOM_infra_end
Expand Down Expand Up @@ -108,6 +108,13 @@ subroutine Get_PEList(pelist, name, commID)
call mpp_get_current_pelist(pelist, name, commiD)
end subroutine Get_PEList

!> Sync the PEs at a defined point in the model
subroutine sync_PEs(pelist)
integer, optional, intent(in) :: pelist(:) !< The list of PEs to be synced

call mpp_sync(pelist)
end subroutine sync_PEs

!> Communicate a 1-D array of character strings from one PE to others
subroutine broadcast_char(dat, length, from_PE, PElist, blocking)
character(len=*), intent(inout) :: dat(:) !< The data to communicate and destination
Expand Down
9 changes: 8 additions & 1 deletion config_src/infra/FMS2/MOM_coms_infra.F90
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ module MOM_coms_infra

implicit none ; private

public :: PE_here, root_PE, num_PEs, set_rootPE, Set_PElist, Get_PElist
public :: PE_here, root_PE, num_PEs, set_rootPE, Set_PElist, Get_PElist, sync_PEs
public :: broadcast, sum_across_PEs, min_across_PEs, max_across_PEs
public :: any_across_PEs, all_across_PEs
public :: field_chksum, MOM_infra_init, MOM_infra_end
Expand Down Expand Up @@ -108,6 +108,13 @@ subroutine Get_PEList(pelist, name, commID)
call mpp_get_current_pelist(pelist, name, commiD)
end subroutine Get_PEList

!> Sync the PEs at a defined point in the model
subroutine sync_PEs(pelist)
integer, optional, intent(in) :: pelist(:) !< The list of PEs to be synced

call mpp_sync(pelist)
end subroutine sync_PEs

!> Communicate a 1-D array of character strings from one PE to others
subroutine broadcast_char(dat, length, from_PE, PElist, blocking)
character(len=*), intent(inout) :: dat(:) !< The data to communicate and destination
Expand Down
Loading

0 comments on commit 9292b58

Please sign in to comment.