Skip to content

Commit

Permalink
(+) Refactor of MOM_file_parser
Browse files Browse the repository at this point in the history
This patch includes several minor changes to the MOM_file_parser and
supporting modules in order to accommodate stronger unit testing.

It includes the following API changes:

- Removal of `static_value` from `get_param`

- Redefined `link_parameter` and `parameter_block` as private

- New functions: `all_across_PEs()`, `any_across_PEs()`

`static_value` was not used in any known experiments (outside of
internal GFDL testing), and the two derived types describe internal
operations within `MOM_file_parser`, so we do not expect any disruptions
from these changes.

A detailed summary of the changes are listed below.

- `assert()` is now used to detect same files with different IO units.

  Detection of reopenend files of the same name but different IO unit
  has been changed from `MOM_error(FATAL, ...)` to `assert()`, to
  reflect that this should be a logically impossible result.

- Bugfix: Reopened files are now reported to all PEs.

  If an open file is re-opened, then only the root PE will detect this
  and will `return` immediately.  However, the others will proceed into
  `populate_param_data` and will get stuck in a broadcast waiting for
  root.

  We fix this by communicating the reopened state to all PEs and allow
  all ranks to return before re-processing the data.

  Note that this could also be resolved by allowing all ranks to track
  IO unit numbers, but for now we do not attempt to change this
  behavior.

- `newunit=` used to generate parameter file IO unit

  The parameter IO unit is now generated by `newunit=` rather than an
  explicit search for an unused IO unit.  Note that this is a Fortran
  2008 feature.

  Testing around available IO units has also been removed.

- Removal of generic IO error handling

  Generic "IO error" tests, and corresponding `err=` arguments, have
  been removed in most cases.  We now rely on the Fortran runtime to
  provide diagnostics on these errors, which should typically exceed any
  information that MOM6 could provide.

- Removal of purported `namelist` support

  There were several blocks of code provided to support namelist syntax,
  but did not appear to be working, nor was there any known instance of
  it being used by anyone, so it has been removed.

- `#define/undef/=` syntax testing across ranks

  Previously, only the root PE would test for consistency of the
  #define-like syntax, even though all ranks have this information.
  This required a second, awkwardly placed syntax test later in the
  subroutine.

  This test is redefined to run over all ranks, and the subsequent test
  has been removed.

- `define/override` test reordering

  The `found_override` test when coupled to a `#define`-like declaration
  was unreachable due to the presence of an even stronger test related
  to valid syntax.

  This test has been moved to provide more detailed information about
  the nature of the error.

- `link_parameter`, `parameter_block` defined as private

  Internal derived types of `MOM_file_parser` are redefined as private.
  This preserves the integrity of instances of these types, and also
  prevents creation of implicit object code required to access them
  externally.

- Removal of `static_value` from `get_param` interface

  The `static_value` argument of `get_param` has been removed, since it
  is functionally equivalent to `default`.  While this is an API change,
  there is no known case of anyone using this argument.

- The `param_type%doc` fields are now properly deallocated after closed.

- Quotes have been added around some filename error warnings, to help
  detect issues related to whitespace.

- `any_across_PEs` and `all_across_PEs`

   New functions for calling `any()` and `all()` across PE ranks have
   been added.  Behavior is in line with other functions, such as
   `min_across_PEs`.
  • Loading branch information
marshallward committed Feb 28, 2022
1 parent a468bee commit 9caa701
Show file tree
Hide file tree
Showing 6 changed files with 123 additions and 137 deletions.
31 changes: 31 additions & 0 deletions config_src/infra/FMS1/MOM_coms_infra.F90
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ module MOM_coms_infra

public :: PE_here, root_PE, num_PEs, set_rootPE, Set_PElist, Get_PElist
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

! This module provides interfaces to the non-domain-oriented communication
Expand Down Expand Up @@ -438,6 +439,36 @@ subroutine min_across_PEs_real_1d(field, length, pelist)
call mpp_min(field, length, pelist)
end subroutine min_across_PEs_real_1d

!> Implementation of any() intrinsic across PEs
function any_across_PEs(field, pelist)
logical, intent(in) :: field !< Local PE value
integer, optional, intent(in) :: pelist(:) !< List of PEs to work with
logical :: any_across_PEs

integer :: field_flag

! FMS1 does not support logical collectives, so integer flags are used.
field_flag = 0
if (field) field_flag = 1
call max_across_PEs(field_flag, pelist)
any_across_PEs = (field_flag > 0)
end function any_across_PEs

!> Implementation of all() intrinsic across PEs
function all_across_PEs(field, pelist)
logical, intent(in) :: field !< Local PE value
integer, optional, intent(in) :: pelist(:) !< List of PEs to work with
logical :: all_across_PEs

integer :: field_flag

! FMS1 does not support logical collectives, so integer flags are used.
field_flag = 0
if (field) field_flag = 1
call min_across_PEs(field_flag, pelist)
all_across_PEs = (field_flag > 0)
end function all_across_PEs

!> Initialize the model framework, including PE communication over a designated communicator.
!! If no communicator ID is provided, the framework's default communicator is used.
subroutine MOM_infra_init(localcomm)
Expand Down
31 changes: 31 additions & 0 deletions config_src/infra/FMS2/MOM_coms_infra.F90
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ module MOM_coms_infra

public :: PE_here, root_PE, num_PEs, set_rootPE, Set_PElist, Get_PElist
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

! This module provides interfaces to the non-domain-oriented communication
Expand Down Expand Up @@ -438,6 +439,36 @@ subroutine min_across_PEs_real_1d(field, length, pelist)
call mpp_min(field, length, pelist)
end subroutine min_across_PEs_real_1d

!> Implementation of any() intrinsic across PEs
function any_across_PEs(field, pelist)
logical, intent(in) :: field !< Local PE value
integer, optional, intent(in) :: pelist(:) !< List of PEs to work with
logical :: any_across_PEs

integer :: field_flag

! FMS1 does not support logical collectives, so integer flags are used.
field_flag = 0
if (field) field_flag = 1
call max_across_PEs(field_flag, pelist)
any_across_PEs = (field_flag > 0)
end function any_across_PEs

!> Implementation of all() intrinsic across PEs
function all_across_PEs(field, pelist)
logical, intent(in) :: field !< Local PE value
integer, optional, intent(in) :: pelist(:) !< List of PEs to work with
logical :: all_across_PEs

integer :: field_flag

! FMS1 does not support logical collectives, so integer flags are used.
field_flag = 0
if (field) field_flag = 1
call min_across_PEs(field_flag, pelist)
all_across_PEs = (field_flag > 0)
end function all_across_PEs

!> Initialize the model framework, including PE communication over a designated communicator.
!! If no communicator ID is provided, the framework's default communicator is used.
subroutine MOM_infra_init(localcomm)
Expand Down
2 changes: 1 addition & 1 deletion src/core/MOM_verticalGrid.F90
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ subroutine verticalGridInit( param_file, GV, US )
! Here NK_ is a macro, while nk is a variable.
call get_param(param_file, mdl, "NK", nk, &
"The number of model layers.", units="nondim", &
static_value=NK_)
default=NK_)
if (nk /= NK_) call MOM_error(FATAL, "verticalGridInit: " // &
"Mismatched number of layers NK_ between MOM_memory.h and param_file")

Expand Down
2 changes: 2 additions & 0 deletions src/framework/MOM_coms.F90
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@ module MOM_coms
use MOM_coms_infra, only : PE_here, root_PE, num_PEs, set_rootPE, Set_PElist, Get_PElist
use MOM_coms_infra, only : broadcast, field_chksum, MOM_infra_init, MOM_infra_end
use MOM_coms_infra, only : sum_across_PEs, max_across_PEs, min_across_PEs
use MOM_coms_infra, only : all_across_PEs, any_across_PEs
use MOM_error_handler, only : MOM_error, MOM_mesg, FATAL, WARNING

implicit none ; private

public :: PE_here, root_PE, num_PEs, MOM_infra_init, MOM_infra_end
public :: broadcast, sum_across_PEs, min_across_PEs, max_across_PEs, field_chksum
public :: all_across_PEs, any_across_PEs
public :: set_PElist, Get_PElist, Set_rootPE
public :: reproducing_sum, reproducing_sum_EFP, EFP_sum_across_PEs, EFP_list_sum_across_PEs
public :: EFP_plus, EFP_minus, EFP_to_real, real_to_EFP, EFP_real_diff
Expand Down
8 changes: 4 additions & 4 deletions src/framework/MOM_domains.F90
Original file line number Diff line number Diff line change
Expand Up @@ -220,11 +220,11 @@ subroutine MOM_domains_init(MOM_dom, param_file, symmetric, static_memory, &
call get_param(param_file, mdl, "NIGLOBAL", n_global(1), &
"The total number of thickness grid points in the x-direction in the physical "//&
"domain. With STATIC_MEMORY_ this is set in "//trim(inc_nm)//" at compile time.", &
static_value=NIGLOBAL)
default=NIGLOBAL)
call get_param(param_file, mdl, "NJGLOBAL", n_global(2), &
"The total number of thickness grid points in the y-direction in the physical "//&
"domain. With STATIC_MEMORY_ this is set in "//trim(inc_nm)//" at compile time.", &
static_value=NJGLOBAL)
default=NJGLOBAL)
if (n_global(1) /= NIGLOBAL) call MOM_error(FATAL,"MOM_domains_init: " // &
"static mismatch for NIGLOBAL_ domain size. Header file does not match input namelist")
if (n_global(2) /= NJGLOBAL) call MOM_error(FATAL,"MOM_domains_init: " // &
Expand Down Expand Up @@ -256,11 +256,11 @@ subroutine MOM_domains_init(MOM_dom, param_file, symmetric, static_memory, &
call get_param(param_file, mdl, trim(nihalo_nm), n_halo(1), &
"The number of halo points on each side in the x-direction. How this is set "//&
"varies with the calling component and static or dynamic memory configuration.", &
default=nihalo_dflt, static_value=nihalo_dflt)
default=nihalo_dflt)
call get_param(param_file, mdl, trim(njhalo_nm), n_halo(2), &
"The number of halo points on each side in the y-direction. How this is set "//&
"varies with the calling component and static or dynamic memory configuration.", &
default=njhalo_dflt, static_value=njhalo_dflt)
default=njhalo_dflt)
if (present(min_halo)) then
n_halo(1) = max(n_halo(1), min_halo(1))
min_halo(1) = n_halo(1)
Expand Down
Loading

0 comments on commit 9caa701

Please sign in to comment.