Skip to content

Commit

Permalink
Fix: Modern diag manager. Catch missing_values without masks (NOAA-GF…
Browse files Browse the repository at this point in the history 
…DL#1481)
  • Loading branch information
@uramirez8707
uramirez8707 authored and rem1776 committed May 1, 2024
1 parent a06ca86 commit a13ef08
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 8 deletions.
13 changes: 7 additions & 6 deletions diag_manager/fms_diag_object.F90
View file
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,7 @@ function fms_diag_do_reduction(this, field_data, diag_field_id, oor_mask, weight
!! in blocks
type(time_type), intent(in), optional :: time !< Current time

character(len=50) :: error_msg !< Error message to check
character(len=150) :: error_msg !< Error message to check
!TODO Mostly everything
#ifdef use_yaml
type(fmsDiagField_type), pointer :: field_ptr !< Pointer to the field's object
Expand Down Expand Up @@ -1032,26 +1032,27 @@ function fms_diag_do_reduction(this, field_data, diag_field_id, oor_mask, weight
endif
case (time_sum)
error_msg = buffer_ptr%do_time_sum_wrapper(field_data, oor_mask, field_ptr%get_var_is_masked(), &
field_ptr%get_mask_variant(), bounds_in, bounds_out, missing_value)
field_ptr%get_mask_variant(), bounds_in, bounds_out, missing_value, field_ptr%has_missing_value())
if (trim(error_msg) .ne. "") then
return
endif
case (time_average)
error_msg = buffer_ptr%do_time_sum_wrapper(field_data, oor_mask, field_ptr%get_var_is_masked(), &
field_ptr%get_mask_variant(), bounds_in, bounds_out, missing_value)
field_ptr%get_mask_variant(), bounds_in, bounds_out, missing_value, field_ptr%has_missing_value())
if (trim(error_msg) .ne. "") then
return
endif
case (time_power)
error_msg = buffer_ptr%do_time_sum_wrapper(field_data, oor_mask, field_ptr%get_var_is_masked(), &
field_ptr%get_mask_variant(), bounds_in, bounds_out, missing_value, &
field_ptr%get_mask_variant(), bounds_in, bounds_out, missing_value, field_ptr%has_missing_value(), &
pow_value=field_yaml_ptr%get_pow_value())
if (trim(error_msg) .ne. "") then
return
endif
case (time_rms)
error_msg = buffer_ptr%do_time_sum_wrapper(field_data, oor_mask, field_ptr%get_var_is_masked(), &
field_ptr%get_mask_variant(), bounds_in, bounds_out, missing_value, pow_value = 2)
field_ptr%get_mask_variant(), bounds_in, bounds_out, missing_value, field_ptr%has_missing_value(), &
pow_value = 2)
if (trim(error_msg) .ne. "") then
return
endif
Expand All @@ -1061,7 +1062,7 @@ function fms_diag_do_reduction(this, field_data, diag_field_id, oor_mask, weight
! sets the diurnal index for reduction within the buffer object
call buffer_ptr%set_diurnal_section_index(time)
error_msg = buffer_ptr%do_time_sum_wrapper(field_data, oor_mask, field_ptr%get_var_is_masked(), &
field_ptr%get_mask_variant(), bounds_in, bounds_out, missing_value)
field_ptr%get_mask_variant(), bounds_in, bounds_out, missing_value, field_ptr%has_missing_value())
if (trim(error_msg) .ne. "") then
return
endif
Expand Down
14 changes: 12 additions & 2 deletions diag_manager/fms_diag_output_buffer.F90
View file
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,7 @@ end function do_time_max_wrapper
!> @brief Does the time_sum reduction method on the buffer object
!! @return Error message if the math was not successful
function do_time_sum_wrapper(this, field_data, mask, is_masked, mask_variant, bounds_in, bounds_out, missing_value, &
pow_value) &
has_missing_value, pow_value) &
result(err_msg)
class(fmsDiagOutputBuffer_type), intent(inout) :: this !< buffer object to write
class(*), intent(in) :: field_data(:,:,:,:) !< Buffer data for current time
Expand All @@ -660,17 +660,23 @@ function do_time_sum_wrapper(this, field_data, mask, is_masked, mask_variant, bo
logical, intent(in) :: is_masked !< .True. if the field has a mask
logical, intent(in) :: mask_variant !< .True. if the mask changes over time
real(kind=r8_kind), intent(in) :: missing_value !< Missing_value for data points that are masked
logical, intent(in) :: has_missing_value !< .True. if the field was registered with
!! a missing value
integer, optional, intent(in) :: pow_value !< power value, will calculate field_data^pow
!! before adding to buffer should only be
!! present if using pow reduction method
character(len=50) :: err_msg
character(len=150) :: err_msg

!TODO This will be expanded for integers
err_msg = ""
select type (output_buffer => this%buffer)
type is (real(kind=r8_kind))
select type (field_data)
type is (real(kind=r8_kind))
if (.not. is_masked) then
if (any(field_data .eq. missing_value)) &
err_msg = "You cannot pass data with missing values without masking them!"
endif
call do_time_sum_update(output_buffer, this%weight_sum, field_data, mask, is_masked, mask_variant, &
bounds_in, bounds_out, missing_value, this%diurnal_section, &
pow=pow_value)
Expand All @@ -680,6 +686,10 @@ function do_time_sum_wrapper(this, field_data, mask, is_masked, mask_variant, bo
type is (real(kind=r4_kind))
select type (field_data)
type is (real(kind=r4_kind))
if (.not. is_masked) then
if (any(field_data .eq. missing_value)) &
err_msg = "You cannot pass data with missing values without masking them!"
endif
call do_time_sum_update(output_buffer, this%weight_sum, field_data, mask, is_masked, mask_variant, &
bounds_in, bounds_out, real(missing_value, kind=r4_kind), &
this%diurnal_section, pow=pow_value)
Expand Down
8 changes: 8 additions & 0 deletions test_fms/diag_manager/test_reduction_methods.F90
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ program test_reduction_methods
integer :: id_var0 !< diag_field id for 0d var
integer :: id_var1 !< diag_field id for 1d var
integer :: id_var2 !< diag_field id for 2d var
integer :: id_var2missing !< diag_field id for a var that is not masked but has missing
!! values passed into send_data
integer :: id_var2c !< diag_field id for 2d var_corner
integer :: id_var3 !< diag_field id for 3d var
integer :: id_var4 !< diag_field id for 4d var
Expand Down Expand Up @@ -177,6 +179,8 @@ program test_reduction_methods
'mullions', missing_value = missing_value)
id_var2 = register_diag_field ('ocn_mod', 'var2', (/id_x, id_y/), Time, 'Var2d', &
'mullions', missing_value = missing_value)
id_var2missing = register_diag_field ('ocn_mod', 'var2missing', (/id_x, id_y/), Time, 'Var2d', &
'mullions', missing_value = missing_value)
id_var2c = register_diag_field ('ocn_mod', 'var2c', (/id_xc, id_yc/), Time, 'Var2d corner', &
'mullions', missing_value = missing_value)
id_var3 = register_diag_field ('ocn_mod', 'var3', (/id_x, id_y, id_z/), Time, 'Var3d', &
Expand All @@ -198,6 +202,10 @@ program test_reduction_methods
call set_buffer(cdata, i)
call set_buffer(cdata_corner, i)

! This is passing in the data with missing values, but the variable is not masked.
! An error is expected in this case.
used = send_data(id_var2missing, cdata(:,:,1,1)*0_r8_kind + missing_value, Time)

used = send_data(id_var2c, cdata_corner(:,:,1,1), Time)
used = send_data(id_var0, cdata(1,1,1,1), Time)

Expand Down
20 changes: 20 additions & 0 deletions test_fms/diag_manager/test_time_avg.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,5 +191,25 @@ test_expect_success "Running diag_manager with "avg" reduction method with halo
test_expect_success "Checking answers for the "avg" reduction method with halo output with real mask (test $my_test_count)" '
mpirun -n 1 ../check_time_avg
'

cat <<_EOF > diag_table.yaml
title: test_avg
base_date: 2 1 1 0 0 0
diag_files:
- file_name: test_failure
time_units: hours
unlimdim: time
freq: 6 hours
varlist:
- module: ocn_mod
var_name: var2missing
reduction: average
kind: r4
_EOF

my_test_count=`expr $my_test_count + 1`
test_expect_failure "Fail if passing in missing_values without masking them (test $my_test_count)" '
mpirun -n 6 ../test_reduction_methods
'
fi
test_done

0 comments on commit a13ef08

Please sign in to comment.