From 41f59150608302367fd8b2b37a3f33589460d546 Mon Sep 17 00:00:00 2001 From: uramirez8707 <49168881+uramirez8707@users.noreply.github.com> Date: Fri, 29 Mar 2024 10:04:03 -0400 Subject: [PATCH] fix: modern_diag_manager empty files and non registered fields (#1482) --- diag_manager/fms_diag_file_object.F90 | 81 +++++++++++++++++++++++++++------ diag_manager/fms_diag_object.F90 | 7 +-- diag_manager/fms_diag_output_buffer.F90 | 10 ++-- diag_manager/fms_diag_yaml.F90 | 14 ++++-- test_fms/diag_manager/test_time_none.sh | 55 ++++++++++++++++++++++ 5 files changed, 141 insertions(+), 26 deletions(-) diff --git a/diag_manager/fms_diag_file_object.F90 b/diag_manager/fms_diag_file_object.F90 index 2ea70dfb..8b6b2cbd 100644 --- a/diag_manager/fms_diag_file_object.F90 +++ b/diag_manager/fms_diag_file_object.F90 @@ -156,6 +156,7 @@ type :: fmsDiagFile_type procedure, public :: dump_file_obj procedure, public :: get_buffer_ids procedure, public :: get_number_of_buffers + procedure, public :: has_send_data_been_called end type fmsDiagFile_type type, extends (fmsDiagFile_type) :: subRegionalFile_type @@ -1323,12 +1324,6 @@ subroutine write_field_data(this, field_obj, buffer_obj) else diag_file%data_has_been_written = .true. has_diurnal = buffer_obj%get_diurnal_sample_size() .gt. 1 - if (.not. buffer_obj%is_there_data_to_write()) then - ! Only print the error message once - if (diag_file%unlim_dimension_level .eq. 1) & - call mpp_error(NOTE, "Send data was never called. Writing fill values for variable "//& - field_obj%get_varname()//" in mod "//field_obj%get_modname()) - endif call buffer_obj%write_buffer(fms2io_fileobj, & unlim_dim_level=diag_file%unlim_dimension_level, is_diurnal=has_diurnal) endif @@ -1350,17 +1345,34 @@ 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) - class(fmsDiagFileContainer_type), intent(in), target :: this !< The file object - TYPE(time_type), intent(in) :: time_step !< Current model step time +logical function is_time_to_write(this, time_step, output_buffers) + 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! 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) & - call mpp_error(FATAL, this%FMS_diag_file%get_file_fname()//& - &": Diag_manager_mod:: You skipped a time_step. Be sure that diag_send_complete is called at every time step "& - &" needed by the file.") + if (time_step > this%FMS_diag_file%next_next_output) then + if (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. 1) then + call mpp_error(NOTE, this%FMS_diag_file%get_file_fname()//& + ": diag_manager_mod: This file does not have any variables registered. Fill values will be written") + this%FMS_diag_file%data_has_been_written = .true. + endif + is_time_to_write =.false. + else + !! Only fail if send data has actually been called for at least one variable + if (this%FMS_diag_file%has_send_data_been_called(output_buffers, .false.)) & + call mpp_error(FATAL, this%FMS_diag_file%get_file_fname()//& + ": diag_manager_mod: You skipped a time_step. Be sure that diag_send_complete is called at every "//& + "time_step needed by the file.") + is_time_to_write =.false. + endif + endif else is_time_to_write = .false. if (this%FMS_diag_file%is_static) then @@ -1663,8 +1675,12 @@ subroutine write_axis_data(this, diag_axis) end subroutine write_axis_data !< @brief Closes the diag_file -subroutine close_diag_file(this) - class(fmsDiagFileContainer_type), intent(inout), target :: this !< The file object +subroutine close_diag_file(this, output_buffers, diag_fields) + class(fmsDiagFileContainer_type), intent(inout), target :: this !< The file object + type(fmsDiagOutputBuffer_type), intent(in) :: output_buffers(:) !< Array of output buffers + !! This is needed for error checking + type(fmsDiagField_type), intent(in), optional :: diag_fields(:) !< Array of diag fields + !! This is needed for error checking if (.not. this%FMS_diag_file%is_file_open) return @@ -1690,6 +1706,8 @@ subroutine close_diag_file(this) else this%FMS_diag_file%next_close = diag_time_inc(this%FMS_diag_file%next_close, VERY_LARGE_FILE_FREQ, DIAG_DAYS) endif + + if (this%FMS_diag_file%has_send_data_been_called(output_buffers, .True., diag_fields)) return end subroutine close_diag_file !> \brief Gets the buffer_id list from the file object @@ -1708,5 +1726,38 @@ pure function get_number_of_buffers(this) get_number_of_buffers = this%number_of_buffers end function get_number_of_buffers +!> @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) & +result(rslt) + class(fmsDiagFile_type), intent(in) :: this !< file object + type(fmsDiagOutputBuffer_type), intent(in), target :: output_buffers(:) !< Array of output buffers + logical, intent(in) :: print_warnings !< .True. if printing warnings + type(fmsDiagField_type), intent(in), optional :: diag_fields(:) !< Array of diag fields + + logical :: rslt + integer :: i !< For do loops + integer :: field_id !< Field id + + rslt = .false. + + if (print_warnings) then + do i = 1, this%number_of_buffers + if (.not. output_buffers(this%buffer_ids(i))%is_there_data_to_write()) then + field_id = output_buffers(this%buffer_ids(i))%get_field_id() + call mpp_error(NOTE, "Send data was never called for field:"//& + trim(diag_fields(field_id)%get_varname())//" mod: "//trim(diag_fields(field_id)%get_modname())//& + " in file: "//trim(this%get_file_fname())//". Writting FILL VALUES!") + endif + enddo + else + do i = 1, this%number_of_buffers + if (output_buffers(this%buffer_ids(i))%is_there_data_to_write()) then + rslt = .true. + return + endif + enddo + endif +end function has_send_data_been_called #endif end module fms_diag_file_object_mod diff --git a/diag_manager/fms_diag_object.F90 b/diag_manager/fms_diag_object.F90 index 12c33e08..9856f412 100644 --- a/diag_manager/fms_diag_object.F90 +++ b/diag_manager/fms_diag_object.F90 @@ -825,7 +825,7 @@ subroutine fms_diag_do_io(this, end_time) call diag_file%increase_unlim_dimension_level() endif - finish_writing = diag_file%is_time_to_write(model_time) + finish_writing = diag_file%is_time_to_write(model_time, this%FMS_diag_output_buffers) ! finish reduction method if its time to write buff_ids = diag_file%FMS_diag_file%get_buffer_ids() @@ -863,10 +863,11 @@ subroutine fms_diag_do_io(this, end_time) call diag_file%update_next_write(model_time) call diag_file%update_current_new_file_freq_index(model_time) call diag_file%increase_unlim_dimension_level() - if (diag_file%is_time_to_close_file(model_time)) call diag_file%close_diag_file() + if (diag_file%is_time_to_close_file(model_time)) call diag_file%close_diag_file(this%FMS_diag_output_buffers, & + diag_fields = this%FMS_diag_fields) else if (force_write) then call diag_file%write_time_data() - call diag_file%close_diag_file() + call diag_file%close_diag_file(this%FMS_diag_output_buffers, diag_fields = this%FMS_diag_fields) endif enddo #endif diff --git a/diag_manager/fms_diag_output_buffer.F90 b/diag_manager/fms_diag_output_buffer.F90 index 7c8b5a4a..a2bad476 100644 --- a/diag_manager/fms_diag_output_buffer.F90 +++ b/diag_manager/fms_diag_output_buffer.F90 @@ -61,7 +61,7 @@ type :: fmsDiagOutputBuffer_type !! ie. diurnal24 = sample size of 24 integer :: diurnal_section= -1 !< the diurnal section (ie 5th index) calculated from the current model !! time and sample size if using a diurnal reduction - logical :: send_data_called !< .True. if send_data has been called + logical, allocatable :: send_data_called !< .True. if send_data has been called type(time_type) :: time !< The last time the data was received type(time_type) :: next_output !< The next time to output the data @@ -826,11 +826,15 @@ end subroutine get_remapped_diurnal_data !! @return .true. if there is data to write function is_there_data_to_write(this) & result(res) - class(fmsDiagOutputBuffer_type), intent(inout) :: this !< Buffer object + class(fmsDiagOutputBuffer_type), intent(in) :: this !< Buffer object logical :: res - res = this%send_data_called + if (allocated(this%send_data_called)) then + res = this%send_data_called + else + res = .false. + endif end function !> @brief Determine if it is time to finish the reduction method diff --git a/diag_manager/fms_diag_yaml.F90 b/diag_manager/fms_diag_yaml.F90 index 308243ed..919b37d7 100644 --- a/diag_manager/fms_diag_yaml.F90 +++ b/diag_manager/fms_diag_yaml.F90 @@ -37,7 +37,7 @@ use diag_data_mod, only: DIAG_NULL, DIAG_OCEAN, DIAG_ALL, DIAG_OTHER, set_base middle_time, begin_time, end_time, MAX_STR_LEN use yaml_parser_mod, only: open_and_parse_file, get_value_from_key, get_num_blocks, get_nkeys, & get_block_ids, get_key_value, get_key_ids, get_key_name -use mpp_mod, only: mpp_error, FATAL, mpp_pe, mpp_root_pe, stdout +use mpp_mod, only: mpp_error, FATAL, NOTE, mpp_pe, mpp_root_pe, stdout use, intrinsic :: iso_c_binding, only : c_ptr, c_null_char use fms_string_utils_mod, only: fms_array_to_pointer, fms_find_my_string, fms_sort_this, fms_find_unique, string use platform_mod, only: r4_kind, i4_kind @@ -356,6 +356,7 @@ 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 + character(len=:), allocatable :: filename!< Diag file name (for error messages) if (diag_yaml_module_initialized) return @@ -393,13 +394,16 @@ subroutine diag_yaml_object_init(diag_subset_output) call get_value_from_key(diag_yaml_id, diag_file_ids(i), "write_file", write_file, is_optional=.true.) if(.not. write_file) ignore(i) = .true. + !< If ignoring the file, ignore the fields in that file too! if (.not. ignore(i)) then - !< If ignoring the file, ignore the fields in that file too! - total_nvars = total_nvars + get_total_num_vars(diag_yaml_id, diag_file_ids(i)) - if (total_nvars .ne. 0) then + nvars = get_total_num_vars(diag_yaml_id, diag_file_ids(i)) + total_nvars = total_nvars + nvars + if (nvars .ne. 0) then actual_num_files = actual_num_files + 1 else - ignore(i) = .true. + call diag_get_value_from_key(diag_yaml_id, diag_file_ids(i), "file_name", filename) + call mpp_error(NOTE, "diag_manager_mod:: the file:"//trim(filename)//" has no variables defined. Ignoring!") + ignore(i) = .True. endif endif enddo diff --git a/test_fms/diag_manager/test_time_none.sh b/test_fms/diag_manager/test_time_none.sh index 9840e0c0..421cbfe0 100755 --- a/test_fms/diag_manager/test_time_none.sh +++ b/test_fms/diag_manager/test_time_none.sh @@ -169,5 +169,60 @@ test_expect_success "Running diag_manager with "none" reduction method with halo test_expect_success "Checking answers for the "none" reduction method with halo output with real mask (test $my_test_count)" ' mpirun -n 1 ../check_time_none ' + +cat <<_EOF > diag_table.yaml +title: test_none +base_date: 2 1 1 0 0 0 +diag_files: +- file_name: test_empty_file + time_units: hours + unlimdim: time + freq: 6 hours +_EOF + +my_test_count=`expr $my_test_count + 1` +test_expect_success "Testing diag manager that defined a diag file with no variables (test $my_test_count)" ' + mpirun -n 6 ../test_reduction_methods +' + +cat <<_EOF > diag_table.yaml +title: test_none +base_date: 2 1 1 0 0 0 +diag_files: +- file_name: test_unregistered_data + time_units: hours + unlimdim: time + freq: 6 hours + varlist: + - module: ocn_mod + var_name: something_funny + reduction: none + kind: r4 +_EOF + +my_test_count=`expr $my_test_count + 1` +test_expect_success "Testing diag manager where no variables were registered for a file (test $my_test_count)" ' + mpirun -n 6 ../test_reduction_methods +' + +cat <<_EOF > diag_table.yaml +title: test_none +base_date: 2 1 1 0 0 0 +diag_files: +- file_name: test_send_data_never_called + time_units: hours + unlimdim: time + freq: 6 hours + varlist: + - module: ocn_mod + var_name: IOnASphere + reduction: none + kind: r4 +_EOF + +my_test_count=`expr $my_test_count + 1` +test_expect_success "Testing diag manager where send data was never called for any fields in a file (test $my_test_count)" ' + mpirun -n 6 ../test_reduction_methods + ' fi test_done -- 2.11.4.GIT