Implements reduction: min/max #1295

ganganoaa · 2023-07-19T21:05:49Z

Description

Adds a member routine fms_diag_do_reduction to the fmsDiagObject_type, which implements the reduction min/max.
Updates CMakeLists.txt to include diag_manager/fms_diag_reduction_methods.F90
Updates Makefile.am with fms_diag_reduction_methods.F90 to fix dependencies
Adds member routines get_starting_index and get_ending_index to the fmsDiagSubAxis_type
Adds member routines set_ibounds, set_jbounds, and set_kbounds to the fmsDiagIbounds_type
Updates fmsDiagFile_type to include procedures get_last_output, get_next_output, get_next_next_output, get_no_more_data
Adds fms_diag_reduction_methods_mod (fms_diag_reduction_methods.F90) which packages subroutines compare_two_sets_of_bounds, real_copy_set, check_indices_order, init_mask_3d, fms_diag_update_extremum, update_scalar_extremum, update_array_extremum; they are grouped here because of their proximity of uses. The first four subroutines were originally declared and defined somewhere else and moved here.

Fixes # (issue)

How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Please also note
any relevant details for your test configuration (e.g. compiler, OS). Include
enough information so someone can reproduce your tests.

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
Any dependent changes have been merged and published in downstream modules
New check tests, if applicable, are included
make distcheck passes

…_methods.F90

…he routine fms_diag_do_reduction

…, fms_diag_reduction_methods.F90

…DL#1225)

BREAKING CHANGE: Any code using the global fms module (libFMS.F90) will break as this adds prefixes to all names in that module.

Public member _bounds3D_ of the fmsDiagBoundsHalos_type is changed to private and adds a getter function _get_bounds3D_ to access the memter.

thomas-robinson

This is great progress. There are a few general comments:

Please put as much description as possible into the error messages. There's nothing more frustrating for a user than encountering "Unsupported type". If you can put information about the actual field/file being worked on, that makes it even better.
Name if statements. There is a lot of logic, and it's very difficult to follow. If you do something like this, it will make it much easier:

bigif: if (flag == maximum) then
...
 nested: if (something) then
 endif nested
else bigif
... 
endif bigif

thomas-robinson · 2023-07-24T18:30:57Z

diag_manager/fms_diag_bbox.F90

   END TYPE fmsDiagIbounds_type

   !> @brief Data structure holding starting and ending indices in the I, J, and
   !! K dimensions. It also has extra members related to halo sizes and updated indices
   !! in I and J dimensions.
   type, public :: fmsDiagBoundsHalos_type
      private
-      type(fmsDiagIbounds_type) :: bounds3D !< Holds starting and ending indices of
+      type(fmsDiagIbounds_type), public :: bounds3D !< Holds starting and ending indices of


This should not be public. You can make a get_bounds3d function to get the indexes.

thomas-robinson · 2023-07-24T18:32:05Z

diag_manager/fms_diag_object.F90

@@ -37,6 +39,8 @@ module fms_diag_object_mod
 use fms_diag_output_buffer_mod
 use fms_mod, only: fms_error_handler
 use constants_mod, only: SECONDS_PER_DAY
+use fms_diag_bbox_mod, only: fmsDiagBoundsHalos_type, recondition_indices
+use fms_diag_reduction_methods_mod


Include the only with the list of routines from the module.

Added only: followed by fms_diag_update_extremum

thomas-robinson · 2023-07-24T18:43:50Z

diag_manager/fms_diag_object.F90

-
-      !!TODO: Is check to bounds of current field necessary?
+    !> Allocate buffers of this field variable
+    !call this%allocate_diag_field_output_buffers(field_data, diag_field_id)


should this line be deleted?

We need this line which is temporarily commented out.

thomas-robinson · 2023-07-24T18:44:17Z

diag_manager/fms_diag_object.F90

+    !fms_diag_accept_data = this%fms_diag_do_reduction(field_data, diag_field_id, oor_mask, weight2, &
+      !time, is_in, js_in, ks_in, ie_in, je_in, ke_in, err_msg)


This is commented out. Should it be deleted?

No. We need this line.

thomas-robinson · 2023-07-24T18:52:45Z

diag_manager/fms_diag_object.F90

+    oor_mask_4d => null()
+    oor_mask_4d(1:size(oor_mask,1), 1:size(oor_mask,2), 1:size(oor_mask,3), 1:1) => oor_mask
+
+    do i = 1, size(this%FMS_diag_fields(diag_field_id)%buffer_ids)


Can you label this loop?

buffer_id_loop: do i = 1, size(this%FMS_diag_fields(diag_field_id)%buffer_ids) ... enddo buffer_id_loop

thomas-robinson · 2023-07-25T14:03:26Z

diag_manager/fms_diag_reduction_methods.F90

+
+  !> @brief Updates a chunk of buffer
+  subroutine update_array_extremum(flag, field_data, buffer, mask, sample, recon_bounds, reduced_k_range)
+    integer :: flag !< 0 for minimum; 1 for extremum


use a parameter instead of 0 and 1

thomas-robinson · 2023-07-25T14:03:31Z

diag_manager/fms_diag_reduction_methods.F90

+
+  !> @brief Updates a chunk of buffer
+  subroutine update_array_extremum(flag, field_data, buffer, mask, sample, recon_bounds, reduced_k_range)
+    integer :: flag !< 0 for minimum; 1 for extremum


use a parameter instead of 0 and 1

thomas-robinson · 2023-07-25T14:05:52Z

diag_manager/fms_diag_reduction_methods.F90

+    type is (real(kind=r4_kind))
+      select type (buffer)
+      type is (real(kind=r4_kind))
+        if (flag .eq. 0) then


Please label some of these larger IF statements in this routine. It would make it much easier to follow if they were labeled.

thomas-robinson · 2023-07-25T14:06:47Z

diag_manager/fms_diag_reduction_methods.F90

+    end select
+  end subroutine update_scalar_extremum
+
+  !> @brief Updates a chunk of buffer


This routine and the one above it need more documentation.

Added more information.

thomas-robinson · 2023-07-25T14:08:16Z

diag_manager/fms_diag_reduction_methods.F90

+          end if
+        end if
+      class default
+        call mpp_error( FATAL, "fms_diag_reduction_methods_mod::update_array_extremum type mismatch")


What is mismatched? Please provide more detail in these error messages.

thomas-robinson

This is great progress. There are a few general comments:

Please put as much description as possible into the error messages. There's nothing more frustrating for a user than encountering "Unsupported type". If you can put information about the actual field/file being worked on, that makes it even better.
Name if statements. There is a lot of logic, and it's very difficult to follow. If you do something like this, it will make it much easier:

bigif: if (flag == maximum) then
...
 nested: if (something) then
 endif nested
else bigif
... 
endif bigif

uramirez8707 · 2023-08-01T17:22:44Z

diag_manager/fms_diag_reduction_methods.F90

+    f3 = recon_bounds%get_fjs()
+    f4 = recon_bounds%get_fje()
+
+    if (flag .ne. 3 .and. flag .ne. 4) then


These shouldn't be hard codded

uramirez8707 · 2023-10-05T14:23:52Z

This is included in #1367

ganganoaa added 24 commits July 18, 2023 10:03

Updates reduction method min/max

424e7ec

Updates Makefile.am, fms_diag_axis_object.F90, and fms_diag_reduction…

e2b58e0

…_methods.F90

Removed a data assignment statement before variable declarations in t…

dd496af

…he routine fms_diag_do_reduction

Updates fms_diag_file_object_mod and fms_diag_object_mod

a314839

Updates fms_diag_bbox_mod and fms_diag_object_mod

b3b0e3c

Updates Makefile.am and fms_diag_object_mod

773aca2

Updates Makefile.am

0207a98

Updates fms_diag_reduction_methods_mod

ffec2be

Updates fms_diag_object_mod and fms_diag_reduction_methods_mod

0f79a5b

Updates fms_diag_reduction_methods_mod

a996ce2

Updates fms_diag_object.F90

24d664a

Update fms_diag_object.F90 and fms_diag_reduction_methods.F90

8cbab45

Updates fms_diag_update_extremum_mod

ab9c56f

Updates fms_diag_update_extremum_mod

24c6251

Updates fms_diag_update_extremum_mod

d74666c

Updates fms_diag_update_extremum_mod

158055e

Updates Makefile.am in diag_manager

345f2f9

Updates CMakeLists.txt, diag_manager/Makefile.am, fms_diag_object.F90…

aeb95fe

…, fms_diag_reduction_methods.F90

Updates error strings in fms_diag_reduction_methods.F90

2be3a65

Update fms_diag_object.F90 and fms_diag_reduction_methods.F90

6f89cf1

Updates fms_diag_object.F90

2721805

Updates fms_diag_object.F90 and fms_diag_reduction_methods.F90

a115651

Updates fms_diag_object.F90 and fms_diag_reduction_methods.F90

da59105

Updates fms_diag_reduction_methods.F90

8c14131

ganganoaa requested a review from thomas-robinson as a code owner July 19, 2023 21:05

ganganoaa added 5 commits July 19, 2023 17:21

Updates fms_diag_reduction_methods.F90

823a25c

Updates fms_diag_reduction_methods.F90

a473284

Updates fms_diag_reduction_methods.F90

5e029ab

Fixes mismatch argument type(weight) in routine fms_diag_do_reduction

4fe328f

Updates fms_diag_object_mod

9c024f8

rem1776 and others added 4 commits July 24, 2023 14:18

test: remove stack limit checks in test scripts (NOAA-GFDL#1280)

e13e83f

fix: mpp global arrays test fixes (NOAA-GFDL#1174)

340fce3

feat: update mpp_do_update_ad.fh to resolve JEDI requirement (NOAA-GF…

59df93a

…DL#1225)

chore: add prefixed aliases for libfms routines (NOAA-GFDL#1262)

78671e5

BREAKING CHANGE: Any code using the global fms module (libFMS.F90) will break as this adds prefixes to all names in that module.

ganganoaa requested review from bensonr and rem1776 as code owners July 24, 2023 18:18

ganganoaa added 7 commits July 24, 2023 15:11

Update fms_diag_reduction_methods.F90

4a5ad15

Update fms_diag_bbox.F90

731486b

Public member _bounds3D_ of the fmsDiagBoundsHalos_type is changed to private and adds a getter function _get_bounds3D_ to access the memter.

Update fms_diag_reduction_methods.F90

151c992

Update fms_diag_reduction_methods.F90

2a7e54c

Update fms_diag_reduction_methods.F90

96287fd

Update fms_diag_reduction_methods.F90

d200e1c

Update fms_diag_object.F90

de75dad

thomas-robinson requested changes Jul 25, 2023

View reviewed changes

ganganoaa added 2 commits July 25, 2023 10:18

Update fms_diag_object.F90

2a55ad9

Update fms_diag_reduction_methods.F90

7361947

thomas-robinson requested changes Jul 25, 2023

View reviewed changes

ganganoaa added 8 commits July 25, 2023 11:43

Update fms_diag_object.F90

cd7beeb

Update fms_diag_reduction_methods.F90

10f0891

Update fms_diag_object.F90

840c3c3

Update fms_diag_reduction_methods.F90

4cdff6d

Update fms_diag_object.F90

ddaec5f

Update fms_diag_object.F90

f7f4a40

Update fms_diag_reduction_methods.F90

fd3b88f

Update fms_diag_reduction_methods.F90

0825006

uramirez8707 reviewed Aug 1, 2023

View reviewed changes

This was referenced Aug 7, 2023

More getter routines for fmsDiagFile_type #1253

Closed

modern_diag_manager: fms_diag_do_reduction #1321

Merged

uramirez8707 closed this Oct 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implements reduction: min/max #1295

Implements reduction: min/max #1295

ganganoaa commented Jul 19, 2023 •

edited

Loading

thomas-robinson left a comment

thomas-robinson Jul 24, 2023

ganganoaa Jul 25, 2023

thomas-robinson Jul 24, 2023

ganganoaa Jul 25, 2023

thomas-robinson Jul 24, 2023

ganganoaa Jul 25, 2023

thomas-robinson Jul 24, 2023

ganganoaa Jul 25, 2023

thomas-robinson Jul 24, 2023

ganganoaa Jul 25, 2023

thomas-robinson Jul 25, 2023

ganganoaa Jul 25, 2023

thomas-robinson Jul 25, 2023

ganganoaa Jul 25, 2023

thomas-robinson Jul 25, 2023

ganganoaa Jul 25, 2023

thomas-robinson Jul 25, 2023

ganganoaa Jul 25, 2023

thomas-robinson Jul 25, 2023

ganganoaa Jul 25, 2023

thomas-robinson left a comment

uramirez8707 Aug 1, 2023

uramirez8707 commented Oct 5, 2023

		!fms_diag_accept_data = this%fms_diag_do_reduction(field_data, diag_field_id, oor_mask, weight2, &
		!time, is_in, js_in, ks_in, ie_in, je_in, ke_in, err_msg)

Implements reduction: min/max #1295

Implements reduction: min/max #1295

Conversation

ganganoaa commented Jul 19, 2023 • edited Loading

thomas-robinson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomas-robinson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uramirez8707 commented Oct 5, 2023

ganganoaa commented Jul 19, 2023 •

edited

Loading