fix: Modern_diag_manager 0 days frequencies and mix_snapshot_average_…

…fields deprecation (#1500)

diag_manager/diag_manager.F90

@@ -4169,11 +4169,15 @@ SUBROUTINE diag_manager_init(diag_model_subset, time_init, err_msg)
     END IF
 
     IF ( mix_snapshot_average_fields ) THEN
-       IF ( mpp_pe() == mpp_root_pe() ) THEN
+       IF ( .not. use_modern_diag ) THEN
           CALL error_mesg('diag_manager_mod::diag_manager_init', 'Setting diag_manager_nml variable '//&
                & 'mix_snapshot_average_fields = .TRUE. will cause ERRORS in the time coordinates '//&
                & 'of all time averaged fields.  Strongly recommend setting mix_snapshot_average_fields '//&
-               & '= .FALSE.', WARNING)
+               & '= .FALSE.', NOTE)
+       ELSE
+          CALL error_mesg('diag_manager_mod::diag_manager_init', 'mix_snapshot_average_fields = .TRUE. is not '//&
+               & 'supported if use_modern_diag = .TRUE. Please set mix_snapshot_average_fields '//&
+               & 'to .FALSE. and put instantaneous and averaged fields in seperate files!', FATAL)
        END IF
     END IF
     ALLOCATE(output_fields(max_output_fields))

diag_manager/fms_diag_file_object.F90

@@ -94,7 +94,8 @@ module fms_diag_file_object_mod
   integer :: number_of_axis !< Number of axis in the file
   integer, dimension(:), allocatable :: buffer_ids !< array of buffer ids associated with the file
   integer :: number_of_buffers !< Number of buffers that have been added to the file
-  logical :: time_ops !< .True. if file contains variables that are time_min, time_max, time_average or time_sum
+  logical, allocatable :: time_ops !< .True. if file contains variables that are time_min, time_max, time_average,
+                                   !! or time_sum
   integer :: unlim_dimension_level !< The unlimited dimension level currently being written
   logical :: data_has_been_written !< .True. if data has been written for the current unlimited dimension level
   logical :: is_static !< .True. if the frequency is -1
@@ -119,6 +120,7 @@ module fms_diag_file_object_mod
   procedure, public :: add_start_time
   procedure, public :: set_file_time_ops
   procedure, public :: has_field_ids
+  procedure, public :: get_time_ops
   procedure, public :: get_id
 ! TODO  procedure, public :: get_fileobj ! TODO
 ! TODO  procedure, public :: get_diag_yaml_file ! TODO
@@ -158,6 +160,7 @@ module fms_diag_file_object_mod
  procedure, public :: get_buffer_ids
  procedure, public :: get_number_of_buffers
  procedure, public :: has_send_data_been_called
+ procedure, public :: check_buffer_times
 end type fmsDiagFile_type
 
 type, extends (fmsDiagFile_type) :: subRegionalFile_type
@@ -275,7 +278,6 @@ logical function fms_diag_files_object_init (files_array)
        obj%no_more_data = diag_time_inc(obj%start_time, VERY_LARGE_FILE_FREQ, DIAG_DAYS)
      endif
 
-     obj%time_ops = .false.
      obj%unlim_dimension_level = 0
      obj%is_static = obj%get_file_freq() .eq. -1
      obj%nz_subaxis = 0
@@ -373,21 +375,31 @@ subroutine set_file_time_ops(this, VarYaml, is_static)
 
   !< Go away if the file is static
   if (this%is_static) return
+  if (is_static) return
 
-  if (this%time_ops) then
-    if (is_static) return
-    if (VarYaml%get_var_reduction() .eq. time_none) then
-      call mpp_error(FATAL, "The file: "//this%get_file_fname()//&
-                            " has variables that are time averaged and instantaneous")
-    endif
-  else
+  ! Set time_ops the first time this subroutine it is called
+  if (.not. allocated(this%time_ops)) then
     var_reduct = VarYaml%get_var_reduction()
+
     select case (var_reduct)
-      case (time_average, time_rms, time_max, time_min, time_sum, time_diurnal, time_power)
-        this%time_ops = .true.
+    case (time_average, time_rms, time_max, time_min, time_sum, time_diurnal, time_power)
+      this%time_ops = .true.
+    case (time_none)
+      this%time_ops = .false.
     end select
+
+    return
   endif
 
+  if (this%time_ops) then
+    if (VarYaml%get_var_reduction() .eq. time_none) &
+      call mpp_error(FATAL, "The file: "//this%get_file_fname()//&
+                            " has variables that are time averaged and instantaneous")
+  else
+    if (VarYaml%get_var_reduction() .ne. time_none) &
+      call mpp_error(FATAL, "The file: "//this%get_file_fname()//&
+                            " has variables that are time averaged and instantaneous")
+  endif
 end subroutine set_file_time_ops
 
 !> \brief Logical function to determine if the variable file_metadata_from_model has been allocated or associated
@@ -443,6 +455,19 @@ pure function get_id (this) result (res)
   res = this%id
 end function get_id
 
+!> \brief Returns a copy of the value of time_ops
+!! \return A copy of time_ops
+pure function get_time_ops (this) result (res)
+  class(fmsDiagFile_type), intent(in) :: this !< The file object
+  logical :: res
+
+  if (.not. allocated(this%time_ops)) then
+    res = .false.
+  else
+    res = this%time_ops
+  endif
+end function get_time_ops
+
 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 !! TODO
 !> \brief Returns a copy of the value of fileobj
@@ -1286,7 +1311,7 @@ subroutine write_time_metadata(this)
   call register_variable_attribute(fms2io_fileobj, time_var_name, "calendar", &
     lowercase(trim(calendar)), str_len=len_trim(calendar))
 
-  if (diag_file%time_ops) then
+  if (diag_file%get_time_ops()) then
     call register_variable_attribute(fms2io_fileobj, time_var_name, "bounds", &
       trim(time_var_name)//"_bnds", str_len=len_trim(time_var_name//"_bnds"))
 
@@ -1366,17 +1391,31 @@ logical function is_time_to_close_file (this, time_step)
 end function
 
 !> \brief Determine if it is time to "write" to the file
-logical function is_time_to_write(this, time_step, output_buffers)
+logical function is_time_to_write(this, time_step, output_buffers, do_not_write)
   class(fmsDiagFileContainer_type), intent(inout), target   :: this              !< The file object
   TYPE(time_type),                  intent(in)              :: time_step         !< Current model step time
   type(fmsDiagOutputBuffer_type),   intent(in)              :: output_buffers(:) !< Array of output buffer.
                                                                                  !! This is needed for error messages!
+  logical,                          intent(out)             :: do_not_write      !< .True. only if this is not a new
+                                                                                 !! time step and you are writting
+                                                                                 !! at every time step
 
+  do_not_write = .false.
   if (time_step > this%FMS_diag_file%next_output) then
     is_time_to_write = .true.
     if (this%FMS_diag_file%is_static) return
     if (time_step > this%FMS_diag_file%next_next_output) then
-      if (this%FMS_diag_file%num_registered_fields .eq. 0) then
+      if (this%FMS_diag_file%get_file_freq() .eq. 0) then
+        !! If the diag file is being written at every time step
+        if (time_step .ne. this%FMS_diag_file%next_output) then
+          !! Only write and update the next_output if it is a new time
+          call this%FMS_diag_file%check_buffer_times(output_buffers)
+          this%FMS_diag_file%next_output = time_step
+          this%FMS_diag_file%next_next_output = time_step
+          is_time_to_write = .true.
+        endif
+        return
+      elseif (this%FMS_diag_file%num_registered_fields .eq. 0) then
         !! If no variables have been registered, write a dummy time dimension for the first level
         !! At least one time level is needed for the combiner to work ...
         if (this%FMS_diag_file%unlim_dimension_level .eq. 0) then
@@ -1400,6 +1439,8 @@ logical function is_time_to_write(this, time_step, output_buffers)
     if (this%FMS_diag_file%is_static) then
       ! This is to ensure that static files get finished in the begining of the run
       if (this%FMS_diag_file%unlim_dimension_level .eq. 1) is_time_to_write = .true.
+    else if(this%FMS_diag_file%get_file_freq() .eq. 0) then
+      do_not_write = .true.
     endif
   endif
 end function is_time_to_write
@@ -1437,7 +1478,7 @@ subroutine write_time_data(this)
   !! that at least one time level is written (this is needed for the combiner)
   if (.not. diag_file%data_has_been_written .and. diag_file%unlim_dimension_level .ne. 1) return
 
-  if (diag_file%time_ops) then
+  if (diag_file%get_time_ops()) then
     middle_time = (diag_file%last_output+diag_file%next_output)/2
     dif = get_date_dif(middle_time, get_base_time(), diag_file%get_file_timeunit())
   else
@@ -1447,7 +1488,7 @@ subroutine write_time_data(this)
   call write_data(fms2io_fileobj, diag_file%get_file_unlimdim(), dif, &
     unlim_dim_level=diag_file%unlim_dimension_level)
 
-  if (diag_file%time_ops) then
+  if (diag_file%get_time_ops()) then
     T1 = get_date_dif(diag_file%last_output, get_base_time(), diag_file%get_file_timeunit())
     T2 = get_date_dif(diag_file%next_output, get_base_time(), diag_file%get_file_timeunit())
 
@@ -1788,6 +1829,29 @@ pure function get_number_of_buffers(this)
   get_number_of_buffers = this%number_of_buffers
 end function get_number_of_buffers
 
+!> Check to ensure that send_data was called at the time step for every output buffer in the file
+!! This is only needed when you are output data at every time step
+subroutine check_buffer_times(this, output_buffers)
+  class(fmsDiagFile_type),        intent(in)           :: this              !< file object
+  type(fmsDiagOutputBuffer_type), intent(in), target   :: output_buffers(:) !< Array of output buffers
+
+  integer :: i
+  type(time_type) :: current_buffer_time
+  character(len=:), allocatable :: field_name
+
+  do i = 1, this%number_of_buffers
+    if (i .eq. 1) then
+      current_buffer_time = output_buffers(this%buffer_ids(i))%get_buffer_time()
+      field_name = output_buffers(this%buffer_ids(i))%get_buffer_name()
+    else
+      if (current_buffer_time .ne. output_buffers(this%buffer_ids(i))%get_buffer_time()) &
+        call mpp_error(FATAL, "Send data has not been called at the same time steps for the fields:"//&
+                              field_name//" and "//output_buffers(this%buffer_ids(i))%get_buffer_name()//&
+                              " in file:"//this%get_file_fname())
+    endif
+  enddo
+end subroutine
+
 !> @brief Determine if send_data has been called for any fields in the file. Prints out warnings, if indicated
 !! @return .True. if send_data has been called for any fields in the file
 function has_send_data_been_called(this, output_buffers, print_warnings, diag_fields) &

diag_manager/fms_diag_object.F90

@@ -807,6 +807,7 @@ subroutine fms_diag_do_io(this, end_time)
   real(r8_kind) :: mval !< r8 copy of missing value
   character(len=128) :: error_string !< outputted error string from reducti
   logical :: unlim_dim_was_increased !< .True. if the unlimited dimension index was increased for any of the buffers
+  logical :: do_not_write !< .True. only if this is not a new time step and you are writting at every time step
 
   force_write = .false.
 
@@ -836,7 +837,7 @@ subroutine fms_diag_do_io(this, end_time)
       call diag_file%write_axis_data(this%diag_axis)
     endif
 
-    finish_writing = diag_file%is_time_to_write(model_time, this%FMS_diag_output_buffers)
+    finish_writing = diag_file%is_time_to_write(model_time, this%FMS_diag_output_buffers, do_not_write)
     unlim_dim_was_increased = .false.
 
     ! finish reduction method if its time to write
@@ -850,7 +851,7 @@ subroutine fms_diag_do_io(this, end_time)
       ! Go away if there is no data to write
       if (.not. diag_buff%is_there_data_to_write()) cycle
 
-      if ( diag_buff%is_time_to_finish_reduction(end_time)) then
+      if ( diag_buff%is_time_to_finish_reduction(end_time) .and. .not. do_not_write) then
         ! sets missing value
         mval = diag_field%find_missing_value(missing_val)
         ! time_average and greater values all involve averaging so need to be "finished" before written

diag_manager/fms_diag_output_buffer.F90

@@ -67,6 +67,7 @@ module fms_diag_output_buffer_mod
   type(time_type)       :: next_output        !< The next time to output the data
 
   contains
+  procedure :: get_buffer_name
   procedure :: add_axis_ids
   procedure :: get_axis_ids
   procedure :: set_field_id
@@ -76,6 +77,7 @@ module fms_diag_output_buffer_mod
   procedure :: init_buffer_time
   procedure :: set_next_output
   procedure :: update_buffer_time
+  procedure :: get_buffer_time
   procedure :: is_there_data_to_write
   procedure :: is_time_to_finish_reduction
   procedure :: set_send_data_called
@@ -301,6 +303,17 @@ subroutine initialize_buffer (this, reduction_method, field_name)
 
 end subroutine initialize_buffer
 
+!> @brief Get the name of the field for the output buffer
+!! @return Name of the field for the output buffer
+function get_buffer_name(this) &
+  result(rslt)
+  class(fmsDiagOutputBuffer_type), intent(in) :: this        !< Buffer object
+
+  character(len=:), allocatable :: rslt
+
+  rslt = diag_yaml%diag_fields(this%yaml_id)%get_var_outname()
+end function get_buffer_name
+
 !> @brief Adds the axis ids to the buffer object
 subroutine add_axis_ids(this, axis_ids)
   class(fmsDiagOutputBuffer_type), intent(inout) :: this        !< Buffer object
@@ -400,6 +413,16 @@ subroutine update_buffer_time(this, time)
   endif
 end subroutine update_buffer_time
 
+!> @brief Get the buffer_time from a output buffer object
+!! @return The buffer time
+function get_buffer_time(this) &
+  result(rslt)
+  class(fmsDiagOutputBuffer_type), intent(in) :: this        !< Buffer object
+  type(time_type) :: rslt
+
+  rslt = this%time
+end function get_buffer_time
+
 !> @brief Determine if finished with math
 !! @return this%done_with_math
 function is_done_with_math(this) &

diag_manager/fms_diag_yaml.F90

@@ -368,7 +368,10 @@ subroutine diag_yaml_object_init(diag_subset_output)
   integer              :: file_count       !! The current number of files added to the diag_yaml obj
   logical              :: write_file       !< Flag indicating if the user wants the file to be written
   logical              :: write_var        !< Flag indicating if the user wants the variable to be written
+  logical              :: allow_averages   !< .True. if averages are allowed (the file is not static of you are
+                                           !! outputing data at every frequency)
   character(len=:), allocatable :: filename!< Diag file name (for error messages)
+  logical              :: is_instantaneous !< .True. if the file is instantaneous (i.e no averaging)
 
   if (diag_yaml_module_initialized) return
 
@@ -447,6 +450,8 @@ subroutine diag_yaml_object_init(diag_subset_output)
     file_var_count = 0
     allocate(diag_yaml%diag_files(file_count)%file_varlist(get_total_num_vars(diag_yaml_id, diag_file_ids(i))))
     allocate(diag_yaml%diag_files(file_count)%file_outlist(get_total_num_vars(diag_yaml_id, diag_file_ids(i))))
+    allow_averages = .not. diag_yaml%diag_files(file_count)%file_freq(1) < 1
+    is_instantaneous = .false.
     nvars_loop: do j = 1, nvars
       write_var = .true.
       call get_value_from_key(diag_yaml_id, var_ids(j), "write_var", write_var, is_optional=.true.)
@@ -462,7 +467,7 @@ subroutine diag_yaml_object_init(diag_subset_output)
       diag_yaml%diag_fields(var_count)%var_axes_names = ""
       diag_yaml%diag_fields(var_count)%var_file_is_subregional = diag_yaml%diag_files(file_count)%has_file_sub_region()
 
-      call fill_in_diag_fields(diag_yaml_id, var_ids(j), diag_yaml%diag_fields(var_count))
+      call fill_in_diag_fields(diag_yaml_id, var_ids(j), diag_yaml%diag_fields(var_count), allow_averages)
 
       !> Save the variable name in the diag_file type
       diag_yaml%diag_files(file_count)%file_varlist(file_var_count) = diag_yaml%diag_fields(var_count)%var_varname
@@ -602,10 +607,11 @@ subroutine fill_in_diag_files(diag_yaml_id, diag_file_id, yaml_fileobj)
 
 !> @brief Fills in a diagYamlFilesVar_type with the contents of a variable block in
 !! diag_table.yaml
-subroutine fill_in_diag_fields(diag_file_id, var_id, field)
+subroutine fill_in_diag_fields(diag_file_id, var_id, field, allow_averages)
   integer,                        intent(in)  :: diag_file_id !< Id of the file block in the yaml file
   integer,                        intent(in)  :: var_id       !< Id of the variable block in the yaml file
   type(diagYamlFilesVar_type), intent(inout)  :: field        !< diagYamlFilesVar_type obj to read the contents into
+  logical,                        intent(in)  :: allow_averages !< .True. if averages are allowed for this file
 
   integer :: natt          !< Number of attributes in variable
   integer :: var_att_id(1) !< Id of the variable attribute block
@@ -619,6 +625,13 @@ subroutine fill_in_diag_fields(diag_file_id, var_id, field)
   call diag_get_value_from_key(diag_file_id, var_id, "reduction", buffer)
   call set_field_reduction(field, buffer)
 
+  if (.not. allow_averages) then
+    if (field%var_reduction .ne. time_none) &
+      call mpp_error(FATAL, "The file "//field%var_fname//" can only have variables that have none as "//&
+        "the reduction method because the frequency is either -1 or 0. "//&
+        "Check your diag_table.yaml for the field:"//trim(field%var_varname))
+  endif
+
   call diag_get_value_from_key(diag_file_id, var_id, "module", field%var_module)
   deallocate(buffer)
   call diag_get_value_from_key(diag_file_id, var_id, "kind", buffer)

test_fms/diag_manager/Makefile.am

@@ -33,9 +33,10 @@ check_PROGRAMS = test_diag_manager test_diag_manager_time \
  test_flexible_time test_diag_update_buffer test_reduction_methods check_time_none \
  check_time_min check_time_max check_time_sum check_time_avg test_diag_diurnal check_time_diurnal \
  check_time_pow check_time_rms check_subregional test_cell_measures test_var_masks \
- check_var_masks test_multiple_send_data test_diag_out_yaml
+ check_var_masks test_multiple_send_data test_diag_out_yaml test_output_every_freq
 
 # This is the source code for the test.
+test_output_every_freq_SOURCES = test_output_every_freq.F90
 test_diag_manager_SOURCES = test_diag_manager.F90
 test_diag_manager_time_SOURCES = test_diag_manager_time.F90
 test_diag_update_buffer_SOURCES= test_diag_update_buffer.F90
@@ -68,14 +69,15 @@ SH_LOG_DRIVER = env AM_TAP_AWK='$(AWK)' $(SHELL) \
 # Run the test.
 TESTS = test_diag_manager2.sh test_time_none.sh test_time_min.sh test_time_max.sh test_time_sum.sh \
         test_time_avg.sh test_time_pow.sh test_time_rms.sh test_time_diurnal.sh test_cell_measures.sh \
-        test_subregional.sh test_var_masks.sh test_multiple_send_data.sh
+        test_subregional.sh test_var_masks.sh test_multiple_send_data.sh test_output_every_freq.sh
 
 testing_utils.mod: testing_utils.$(OBJEXT)
 
 # Copy over other needed files to the srcdir
 EXTRA_DIST = test_diag_manager2.sh check_crashes.sh test_time_none.sh test_time_min.sh test_time_max.sh \
              test_time_sum.sh test_time_avg.sh test_time_pow.sh test_time_rms.sh test_time_diurnal.sh \
-             test_cell_measures.sh test_subregional.sh test_var_masks.sh test_multiple_send_data.sh
+             test_cell_measures.sh test_subregional.sh test_var_masks.sh test_multiple_send_data.sh \
+             test_output_every_freq.sh
 
 if USING_YAML
 skipflag=""

test_fms/diag_manager/test_diag_manager2.sh

@@ -560,13 +560,13 @@ diag_files:
   - module: test_diag_manager_mod
     var_name: sstt
     output_name: sstt
-    reduction: average
+    reduction: none
     kind: r4
     long_name: S S T
   - module: test_diag_manager_mod
     var_name: sstt2
     output_name: sstt2
-    reduction: average
+    reduction: none
     kind: r4
     long_name: S S T
     write_var: false