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

+(*)Fix problems with generic_tracer_min_max #615

Merged
merged 1 commit into from
Jul 8, 2024

Conversation

Hallberg-NOAA
Copy link
Member

Revise MOM_generic_tracer_min_max() and array_global_min_max() to fix several problems that had previously been noted, and to make the location arguments optional, reflecting that they are optional in the calling routine. This requires a revision to the order of arguments to MOM_generic_tracer_min_max(). The interface to array_global_min_max() was extensively revised and simplified to replace several previous arguments with a single new ocean_grid_type argument, to make the location arguments optional, and add an optional unscale argument.

This commit fixes a number of problems that had previously been noted with array_global_min_max(), including that:
(1) It actually returns the global minimum and maximum values of the tracer;
(2) It gives a position that is independent of the domain decomposition and grid rotation;
(3) For all-zero arrays it correctly reports 0 for the minimum and maximum;
(4) It properly handles unscaling of the input array;
(5) It does not use a 3-d mask array that is actually just a 2-d mask array;
(6) It is more efficient by grouping the global minimum and maximum calls.

This commit also adds or revises comments to document the units and purpose of the remaining undocumented real variables in MOM_generic_tracer.F90.

These changes were tested and verified to be correct by calling array_global_min_max() for temperatures and salinities from write_energy(), but those changes were not included in this commit.

This commit also adds an optional verbosity argument to g_tracer_flux_init() in config_src/external/GFDL_ocean_BGC/generic_tracer_utils.F90, reflecting a change to ocean_BGC/generic_tracers/generic_tracer_utils.F90 that was adopted in August of 2020, and is uses this new verbosity argument in the call from MOM_generic_flux_init().

While all solutions are bitwise identical, there are changes (corrections) in some diagnostic output, and there are changes to the arguments of publicly visible routines.

@Hallberg-NOAA Hallberg-NOAA added bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request answer-changing A change in results (actual or potential) labels Apr 30, 2024
@Hallberg-NOAA Hallberg-NOAA force-pushed the fix_generic_min_max branch from 42170c1 to 5159127 Compare July 3, 2024 20:37
  Revise MOM_generic_tracer_min_max and array_global_min_max to fix several
problems that had previously been noted, and to make the location arguments
optional, reflecting that they are optional in the calling routine.  This
requires a revision to the order of arguments to MOM_generic_tracer_min_max.
The interface to array_global_min_max was extensively revised and simplified to
replace several previous arguments with a single new ocean_grid_type argument,
to make the location arguments optional, and add an optional unscale argument.

  This commit fixes a number of problems that had previously been noted with
array_global_min_max, including that:
 (1) It actually returns the global minimum and maximum values of the tracer;
 (2) It gives a position that is independent of the domain decomposition
     and grid rotation;
 (3) For all-zero arrays it correctly reports 0 for the minimum and maximum;
 (4) It properly handles unscaling of the input array;
 (5) It does not use a 3-d mask array that is actually just a 2-d mask array;
 (6) It is more efficient by grouping the global minimum and maximum calls.

  This commit also adds or revises comments to document the units and purpose
of the remaining undocumented real variables in MOM_generic_tracer.F90.

  These changes were tested and verified to be correct by calling
array_global_min_max for temperatures and salinities from write_energy, but
those changes were not included in this commit.

  This commit also adds an optional verbosity argument to g_tracer_flux_init in
config_src/external/GFDL_ocean_BGC/generic_tracer_utils.F90, reflecting a change
to ocean_BGC/generic_tracers/generic_tracer_utils.F90 that was adopted in August
of 2020, and is uses this new verbosity argument in the call from
MOM_generic_flux_init.

  While all solutions are bitwise identical, there are changes (corrections) in
some diagnostic output, and there are changes to the arguments of publicly
visible routines.
@Hallberg-NOAA Hallberg-NOAA force-pushed the fix_generic_min_max branch from 5159127 to be544ab Compare July 6, 2024 16:47
Copy link
Member

@marshallward marshallward left a comment

Choose a reason for hiding this comment

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

We've had some more discussion about this PR, and although there may be some performance improvements from intrinsics such as maxval and maxloc, the function itself is currently only used for display of diagnostic output and does not need to be addressed immediately.

It's possible that some codes outside our testing such as BGCs could use this function, so we will open an issue and circle back to it when we get a chance.

@marshallward
Copy link
Member

@marshallward marshallward merged commit f596c81 into NOAA-GFDL:dev/gfdl Jul 8, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
answer-changing A change in results (actual or potential) bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants