Skip to content

Commit

Permalink
Merge pull request #368 from NCAR/get_close_caching
Browse files Browse the repository at this point in the history
Removal of redundant caching in assim_tools_mod.f90
  • Loading branch information
mjs2369 authored Jul 14, 2022
2 parents 9c7d829 + 6dc832c commit cbe0d41
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 37 deletions.
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

0 comments on commit cbe0d41

Please sign in to comment.