Reduce memory usage during GWDO stats estimation#1228
Reduce memory usage during GWDO stats estimation#1228abishekg7 wants to merge 21 commits intoMPAS-Dev:developfrom
Conversation
… statistics are checked to be correct
- box, box_landuse, dxm are now local variables - landuse changed to be of type I1KIND - cleanup
|
@mgduda Thanks for the review! I pushed a set of changes addressing all your comments, and it's also tested on Eris. Will try on Derecho next. |
| integer (kind=I1KIND), dimension(:), pointer :: hlanduse ! Dominant land mask (0 or 1) | ||
| real (kind=RKIND) :: hc ! critical height | ||
|
|
||
There was a problem hiding this comment.
Let's remove the spaces on this line.
| character(len=StrKIND) :: geog_sub_path | ||
| character(len=StrKIND+1) :: geog_data_path ! same as config_geog_data_path, but guaranteed to have a trailing slash | ||
|
|
||
| TYPE(mpas_gwd_tile_type), pointer :: tilesHead ! Pointer to linked list of tiles |
There was a problem hiding this comment.
I'd suggest changing TYPE(mpas_gwd_tile_type) to type (mpas_gwd_tile_type).
| real (kind=RKIND), dimension(:,:), pointer :: box => null() ! Subset of topography covering a grid cell | ||
| real (kind=RKIND), dimension(:,:), pointer :: dxm => null() ! Size (meters) in zonal direction of a grid cell | ||
| real (kind=RKIND) :: box_mean ! Mean value of topography in box | ||
| integer (kind=I1KIND), dimension(:,:), pointer :: box_landuse => null() ! Subset of landuse covering a grid cell |
There was a problem hiding this comment.
It's more of a personal style preference, but what do you think about aligning the comments, e.g.,
real (kind=RKIND), dimension(:,:), pointer :: box => null() ! Subset of topography covering a grid cell
real (kind=RKIND), dimension(:,:), pointer :: dxm => null() ! Size (meters) in zonal direction of a grid cell
real (kind=RKIND) :: box_mean ! Mean value of topography in box
integer (kind=I1KIND), dimension(:,:), pointer :: box_landuse => null() ! Subset of landuse covering a grid cell
Otherwise, perhaps a consistent amount of whitespace between the end of the statement and the ! character would be good.
Also, I think initializing these variables will give them an implicit save attribute, which may not be desirable.
| deallocate(hlanduse) | ||
|
|
||
| iErr = 0 | ||
| iErr = remove_tiles(tilesHead) |
There was a problem hiding this comment.
Just some alternative verbs to consider for this function name that might make it clearer that we're freeing memory associated with all tiles in the linked list: delete, free, deallocate. For example, what if we called this function free_tiles? Or free_tile_list?
|
Replaced by #1235. Closing. |
2 participants
This PR attempts to improve the memory footprint during the computation of Gravity Wave Drag Orography statistics in the pre-processing step.
The previous approach relied on each MPI rank reading all of the topography and land use tiles, and hence would run out of memory before we could fully subscribe to all cores in a node. In the new approach, we only read in one tile at a time and when we encounter a pixel whose data is not already available in a linked list. The
get_boxsubroutine is modified to callget_tile_from_box_pointto check if the current pixel in the box is present in the linked list, and if not, it appends this tile (after reading both topo and land use data) to the head of the list.This PR also changes
box, box_landuse, dxm, nx and nyto be local variables, instead of module variables. This provides a little better readability, along with advantages of thread safety, etc.