Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

infra, FMS_cap: stdout_if_root() #1392

Merged
merged 2 commits into from
May 15, 2021

Conversation

marshallward
Copy link
Collaborator

This patch adds get_root_stdout to the MOM_io API, and points to the
FMS stdout() function in the FMS infra implementations.

This change was required because calls to coupler_type_write_chksums
handles both its own checksums across ranks and its own IO to outunit.
Typically only the root PE will write the result.

The FMS stdout() function would return the designated stdout unit for
the root PE, and the internal etc_unit for other PEs, usually set to
/dev/null.

When MOM_io switched from using the FMS stdout() function to the
stdout unit as defined in iso_fortran_env, this functionality was
lost and every PE would write the same result to stdout.

Normally this was controlled with if (root)-like tests, but this
cannot be used in functions like coupler_type_write_checksums, which
require participation of all PEs.

We decided the only resolution here was to introduce a new function,
get_root_stdout which replicates the original behavior of stdout().

Additional comments:

  • The if (root) checks were retained, since it is presumably still a
    good idea to avoid the write() calls when possible, even to
    /dev/null.

  • This patch only updates the FMS coulper driver, but I would recommend
    that the other driver maintainers review their own calls to this
    function.

@marshallward
Copy link
Collaborator Author

marshallward commented May 6, 2021

This needs to be reviewed by @nikizadehgfdl before being merged.


✔️ Niki has confirmed to me that this is working.

This patch adds `stdout_if_root` to the MOM_io API, and points to the
FMS `stdout()` function in the FMS infra implementations.

This change was required because calls to `coupler_type_write_chksums`
handles both its own checksums across ranks and its own IO to `outunit`.
Typically only the root PE will write the result.

The FMS `stdout()` function would return the designated stdout unit for
the root PE, and the internal `etc_unit` for other PEs, usually set to
`/dev/null`.

When MOM_io switched from using the FMS `stdout()` function to the
`stdout` unit as defined in `iso_fortran_env`, this functionality was
lost and every PE would write the same result to stdout.

Normally this was controlled with `if (root)`-like tests, but this
cannot be used in functions like `coupler_type_write_checksums`, which
require participation of all PEs.

We decided the only resolution here was to introduce a new function,
`stdout_if_root` which replicates the original behavior of `stdout()`.

Additional comments:

* The `if (root)` checks were retained, since it is presumably still a
  good idea to avoid the `write()` calls when possible, even to
  `/dev/null`.

* This patch only updates the FMS coulper driver, but I would recommend
  that the other driver maintainers review their own calls to this
  function.
@marshallward
Copy link
Collaborator Author

PR was updated to rename get_root_stdout to stdout_if_root. Also fixed a bug in FMS2 infra (function was not declared public).

@marshallward marshallward changed the title infra, FMS_cap: get_root_stdout() infra, FMS_cap: stdout_if_root() May 11, 2021
@Hallberg-NOAA
Copy link
Collaborator

This PR has passed pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/12667.

Copy link
Collaborator

@Hallberg-NOAA Hallberg-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes seem sensible enough and should address the problems of excessive checksum output.

@Hallberg-NOAA Hallberg-NOAA merged commit fff9d42 into mom-ocean:dev/gfdl May 15, 2021
@marshallward marshallward deleted the cpl_stdout branch February 3, 2022 14:18
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.

2 participants