Skip to content

Testing: Rotation support for MOM_read_data#1488

Merged
Hallberg-NOAA merged 4 commits into
mom-ocean:dev/gfdlfrom
marshallward:rotate_read_data
Sep 9, 2021
Merged

Testing: Rotation support for MOM_read_data#1488
Hallberg-NOAA merged 4 commits into
mom-ocean:dev/gfdlfrom
marshallward:rotate_read_data

Conversation

@marshallward
Copy link
Copy Markdown
Collaborator

This patch adds an interface and several implementations of
MOM_read_data and MOM_read_vector functions which support rotational
testing.

The MOM_domain_type now carries a scalar, turns, indicating the
number of quarter-turns from the input grid to the model grid, and a
pointer to the original unrotated MOM_domain. If this number is
nonzero, then the input field is read into an array based on the input
grid, and then rotated to a new array based on the model grid. This
final result is returned by the function.

If the domain's turns is zero, then it is assumed to be a call from a
non-rotated domain and no rotation is applied. Functions outside of MOM
(such as calls within drivers) do not apply this rotation.

For the "domain-less" reads of 2d arrays, an explicit turns argument
is supported. This only appears to be necessary in one part of grid
initialization.

This is now the third place where turns is tracked: the first is HI
(horizontal index tracking) of the MOM grid, the second in restart_CS,
and now in MOM_Domain. However, I believe this is a reasonable (if
not necessary) place to track the domains while also avoiding a need for
users to explicitly rotate fields every time read_data is called.

This patch adds an interface and several implementations of
`MOM_read_data` and `MOM_read_vector` functions which support rotational
testing.

The `MOM_domain_type` now carries a scalar, `turns`, indicating the
number of quarter-turns from the input grid to the model grid, and a
pointer to the original unrotated `MOM_domain`.  If this number is
nonzero, then the input field is read into an array based on the input
grid, and then rotated to a new array based on the model grid.  This
final result is returned by the function.

If the domain's `turns` is zero, then it is assumed to be a call from a
non-rotated domain and no rotation is applied.  Functions outside of MOM
(such as calls within drivers) do not apply this rotation.

For the "domain-less" reads of 2d arrays, an explicit `turns` argument
is supported.  This only appears to be necessary in one part of grid
initialization.

This is now the third place where `turns` is tracked: the first is HI
(horizontal index tracking) of the MOM grid, the second in `restart_CS`,
and now in `MOM_Domain`.  However, I believe this is a reasonable (if
not necessary) place to track the domains while also avoiding a need for
users to explicitly rotate fields every time `read_data` is called.
@marshallward
Copy link
Copy Markdown
Collaborator Author

The previous version of this did not include the pointer to the unrotated domain, so was incorrectly trying to use the rotated domain on unrotated inputs.

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 28, 2021

Codecov Report

Merging #1488 (e5a522b) into dev/gfdl (b560b2c) will decrease coverage by 0.02%.
The diff coverage is 14.63%.

Impacted file tree graph

@@             Coverage Diff              @@
##           dev/gfdl    #1488      +/-   ##
============================================
- Coverage     29.03%   29.01%   -0.03%     
============================================
  Files           236      236              
  Lines         71407    71506      +99     
============================================
+ Hits          20736    20750      +14     
- Misses        50671    50756      +85     
Impacted Files Coverage Δ
src/core/MOM.F90 58.60% <0.00%> (ø)
config_src/infra/FMS1/MOM_io_infra.F90 20.59% <5.00%> (ø)
src/framework/MOM_io.F90 22.68% <11.57%> (-1.34%) ⬇️
config_src/infra/FMS1/MOM_domain_infra.F90 37.99% <80.00%> (+0.24%) ⬆️
src/initialization/MOM_grid_initialize.F90 62.37% <100.00%> (ø)

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 b560b2c...e5a522b. Read the comment docs.

@Hallberg-NOAA
Copy link
Copy Markdown
Collaborator

The PGI compiler does not appear to be able to differentiate between the various new interfaces, which is a problem that would have to be resolved before we could merge this PR into dev/gfdl.

@marshallward
Copy link
Copy Markdown
Collaborator Author

marshallward commented Sep 7, 2021

The dual definitions of MOM_read_data in src/framework/MOM_io.F90 and config_src/infra/FMSx/MOM_io_infra.F90 appear to be tripping up the PGI compiler.

Even though we locally redefine the interface to read_data_infra, it looks like PGI cannot quite get there and thinks we've defined two interfaces.

I guess something will need to be renamed. I'd suggest renaming the MOM_read_data in config_src/infra to read_data_infra but I am open to any other suggestions.

This patch removes the `read_data_infra` alias in `MOM_io` to the
infra's identically named `MOM_read_data`, and instead renames the infra
function to `read_field`.

This is primarily done to avert the inability of the PGI compiler to
resolve the alias in the namespace.

It also provides a slightly more consistent namespace:

1. It resembles the `write_field` functions in the infra layer.

2. It avoids reuse of the `read_data` name, used in FMS.

3. It allows the MOM framework layer to preserve its `MOM_` suffix,
   as expected for a layer intended for use within MOM.

4. `read_field` sheds any explicit reference to `MOM_`, helping to
   identify the infra layer as a generic interface to its framework.
@marshallward
Copy link
Copy Markdown
Collaborator Author

As discussed outside the issue, I have renamed the instances of MOM_read_data in MOM_io_infra to read_field. These changes appear to work with PGI.

@Hallberg-NOAA
Copy link
Copy Markdown
Collaborator

This updated PR has passed TC testing and pipeline testing with Pipeline #13545, and I agree with the updated code after visual inspection.

@Hallberg-NOAA Hallberg-NOAA merged commit a316416 into mom-ocean:dev/gfdl Sep 9, 2021
@marshallward marshallward deleted the rotate_read_data branch October 20, 2021 13:01
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.

3 participants