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

Redundant caching in assim_tools_mod.f90 #368

Merged
merged 8 commits into from
Jul 14, 2022
Merged
4 changes: 4 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ individual files.

The changes are now listed with the most recent at the top.

**July 14 2022 :: Performance improvement - removal of redundant caching. Tag: v10.0.3**
- Reduces the runtime by removing redundant caching in the get_close_obs_cached and
get_close_state_cached subroutines in assim_tools_mod.f90

**June 24 2022 :: Bug-fixes for MITgcm_ocean and Var obs converter. Tag: v10.0.2**

- MITgcm_ocean pert_model_copies routine fixed to use the correct variable clamping
Expand Down
44 changes: 8 additions & 36 deletions assimilation_code/modules/assimilation/assim_tools_mod.f90
Original file line number Diff line number Diff line change
Expand Up @@ -332,8 +332,6 @@ subroutine filter_assim(ens_handle, obs_ens_handle, obs_seq, keys, &
real(r8) :: vertvalue_obs_in_localization_coord, whichvert_real
real(r8), allocatable :: close_obs_dist(:)
real(r8), allocatable :: close_state_dist(:)
real(r8), allocatable :: last_close_obs_dist(:)
real(r8), allocatable :: last_close_state_dist(:)

integer(i8) :: state_index
integer(i8), allocatable :: my_state_indx(:)
Expand All @@ -351,8 +349,6 @@ subroutine filter_assim(ens_handle, obs_ens_handle, obs_seq, keys, &
integer :: istatus, localization_unit
integer, allocatable :: close_obs_ind(:)
integer, allocatable :: close_state_ind(:)
integer, allocatable :: last_close_obs_ind(:)
integer, allocatable :: last_close_state_ind(:)
integer, allocatable :: my_obs_kind(:)
integer, allocatable :: my_obs_type(:)
integer, allocatable :: my_state_kind(:)
Expand All @@ -376,19 +372,15 @@ subroutine filter_assim(ens_handle, obs_ens_handle, obs_seq, keys, &

! allocate rather than dump all this on the stack
allocate(close_obs_dist( obs_ens_handle%my_num_vars), &
last_close_obs_dist(obs_ens_handle%my_num_vars), &
close_obs_ind( obs_ens_handle%my_num_vars), &
last_close_obs_ind( obs_ens_handle%my_num_vars), &
vstatus( obs_ens_handle%my_num_vars), &
my_obs_indx( obs_ens_handle%my_num_vars), &
my_obs_kind( obs_ens_handle%my_num_vars), &
my_obs_type( obs_ens_handle%my_num_vars), &
my_obs_loc( obs_ens_handle%my_num_vars))

allocate(close_state_dist( ens_handle%my_num_vars), &
last_close_state_dist(ens_handle%my_num_vars), &
close_state_ind( ens_handle%my_num_vars), &
last_close_state_ind( ens_handle%my_num_vars), &
my_state_indx( ens_handle%my_num_vars), &
my_state_kind( ens_handle%my_num_vars), &
my_state_loc( ens_handle%my_num_vars))
Expand Down Expand Up @@ -544,10 +536,6 @@ subroutine filter_assim(ens_handle, obs_ens_handle, obs_seq, keys, &
last_base_states_loc = set_location_missing()
last_num_close_obs = -1
last_num_close_states = -1
last_close_obs_ind(:) = -1
last_close_state_ind(:) = -1
last_close_obs_dist(:) = 888888.0_r8 ! something big, not small
last_close_state_dist(:) = 888888.0_r8 ! ditto
num_close_obs_cached = 0
num_close_states_cached = 0
num_close_obs_calls_made = 0
Expand Down Expand Up @@ -676,8 +664,8 @@ subroutine filter_assim(ens_handle, obs_ens_handle, obs_seq, keys, &
! Do get_close_obs first, even though state space increments are computed before obs increments.
call get_close_obs_cached(gc_obs, base_obs_loc, base_obs_type, &
my_obs_loc, my_obs_kind, my_obs_type, num_close_obs, close_obs_ind, close_obs_dist, &
ens_handle, last_base_obs_loc, last_num_close_obs, last_close_obs_ind, &
last_close_obs_dist, num_close_obs_cached, num_close_obs_calls_made)
ens_handle, last_base_obs_loc, last_num_close_obs, num_close_obs_cached, &
num_close_obs_calls_made)

! set the cutoff default, keep a copy of the original value, and avoid
! looking up the cutoff in a list if the incoming obs is an identity ob
Expand All @@ -697,8 +685,8 @@ subroutine filter_assim(ens_handle, obs_ens_handle, obs_seq, keys, &
! Find state variables on my process that are close to observation being assimilated
call get_close_state_cached(gc_state, base_obs_loc, base_obs_type, &
my_state_loc, my_state_kind, my_state_indx, num_close_states, close_state_ind, close_state_dist, &
ens_handle, last_base_states_loc, last_num_close_states, last_close_state_ind, &
last_close_state_dist, num_close_states_cached, num_close_states_calls_made)
ens_handle, last_base_states_loc, last_num_close_states, num_close_states_cached, &
num_close_states_calls_made)
!call test_close_obs_dist(close_state_dist, num_close_states, i)

! Loop through to update each of my state variables that is potentially close
Expand Down Expand Up @@ -811,20 +799,16 @@ subroutine filter_assim(ens_handle, obs_ens_handle, obs_seq, keys, &

! deallocate space
deallocate(close_obs_dist, &
last_close_obs_dist, &
my_obs_indx, &
my_obs_kind, &
my_obs_type, &
close_obs_ind, &
last_close_obs_ind, &
vstatus, &
my_obs_loc)

deallocate(close_state_dist, &
last_close_state_dist, &
my_state_indx, &
close_state_ind, &
last_close_state_ind, &
my_state_kind, &
my_state_loc)

Expand Down Expand Up @@ -2575,8 +2559,8 @@ end subroutine get_my_obs_loc

subroutine get_close_obs_cached(gc_obs, base_obs_loc, base_obs_type, &
my_obs_loc, my_obs_kind, my_obs_type, num_close_obs, close_obs_ind, close_obs_dist, &
ens_handle, last_base_obs_loc, last_num_close_obs, last_close_obs_ind, &
last_close_obs_dist, num_close_obs_cached, num_close_obs_calls_made)
ens_handle, last_base_obs_loc, last_num_close_obs, num_close_obs_cached, &
num_close_obs_calls_made)

type(get_close_type), intent(in) :: gc_obs
type(location_type), intent(inout) :: base_obs_loc, my_obs_loc(:)
Expand All @@ -2586,8 +2570,6 @@ subroutine get_close_obs_cached(gc_obs, base_obs_loc, base_obs_type, &
type(ensemble_type), intent(in) :: ens_handle
type(location_type), intent(inout) :: last_base_obs_loc
integer, intent(inout) :: last_num_close_obs
integer, intent(inout) :: last_close_obs_ind(:)
real(r8), intent(inout) :: last_close_obs_dist(:)
integer, intent(inout) :: num_close_obs_cached, num_close_obs_calls_made

! This logic could be arranged to make code less redundant
Expand All @@ -2598,8 +2580,6 @@ subroutine get_close_obs_cached(gc_obs, base_obs_loc, base_obs_type, &
else
if (base_obs_loc == last_base_obs_loc) then
num_close_obs = last_num_close_obs
close_obs_ind(:) = last_close_obs_ind(:)
close_obs_dist(:) = last_close_obs_dist(:)
num_close_obs_cached = num_close_obs_cached + 1
else
call get_close_obs(gc_obs, base_obs_loc, base_obs_type, &
Expand All @@ -2608,8 +2588,6 @@ subroutine get_close_obs_cached(gc_obs, base_obs_loc, base_obs_type, &

last_base_obs_loc = base_obs_loc
last_num_close_obs = num_close_obs
last_close_obs_ind(:) = close_obs_ind(:)
last_close_obs_dist(:) = close_obs_dist(:)
num_close_obs_calls_made = num_close_obs_calls_made +1
endif
endif
Expand All @@ -2622,8 +2600,8 @@ end subroutine get_close_obs_cached

subroutine get_close_state_cached(gc_state, base_obs_loc, base_obs_type, &
my_state_loc, my_state_kind, my_state_indx, num_close_states, close_state_ind, close_state_dist, &
ens_handle, last_base_states_loc, last_num_close_states, last_close_state_ind, &
last_close_state_dist, num_close_states_cached, num_close_states_calls_made)
ens_handle, last_base_states_loc, last_num_close_states, num_close_states_cached, &
num_close_states_calls_made)

type(get_close_type), intent(in) :: gc_state
type(location_type), intent(inout) :: base_obs_loc, my_state_loc(:)
Expand All @@ -2634,8 +2612,6 @@ subroutine get_close_state_cached(gc_state, base_obs_loc, base_obs_type, &
type(ensemble_type), intent(in) :: ens_handle
type(location_type), intent(inout) :: last_base_states_loc
integer, intent(inout) :: last_num_close_states
integer, intent(inout) :: last_close_state_ind(:)
real(r8), intent(inout) :: last_close_state_dist(:)
integer, intent(inout) :: num_close_states_cached, num_close_states_calls_made

! This logic could be arranged to make code less redundant
Expand All @@ -2646,8 +2622,6 @@ subroutine get_close_state_cached(gc_state, base_obs_loc, base_obs_type, &
else
if (base_obs_loc == last_base_states_loc) then
num_close_states = last_num_close_states
close_state_ind(:) = last_close_state_ind(:)
close_state_dist(:) = last_close_state_dist(:)
num_close_states_cached = num_close_states_cached + 1
else
call get_close_state(gc_state, base_obs_loc, base_obs_type, &
Expand All @@ -2656,8 +2630,6 @@ subroutine get_close_state_cached(gc_state, base_obs_loc, base_obs_type, &

last_base_states_loc = base_obs_loc
last_num_close_states = num_close_states
last_close_state_ind(:) = close_state_ind(:)
last_close_state_dist(:) = close_state_dist(:)
num_close_states_calls_made = num_close_states_calls_made +1
endif
endif
Expand Down
2 changes: 1 addition & 1 deletion conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
author = 'Data Assimilation Research Section'

# The full version, including alpha/beta/rc tags
release = '10.0.2'
release = '10.0.3'
master_doc = 'README'

# -- General configuration ---------------------------------------------------
Expand Down