Reduce memory usage during GWDO stats estimation #1235
Conversation
… statistics are checked to be correct
- box, box_landuse, dxm are now local variables - landuse changed to be of type I1KIND - cleanup
|
Tested on Eris (GNU) with 24km grid, and Derecho (GNU) with 15km and 24km. Derecho NVHPC build and run is successful for 15km grid. |
|
Tested on 5km grid as well. |
| ! computes the mean elevation in the array and stores that value in | ||
| ! the module variable 'box_mean'. | ||
| ! the module variable 'box_mean'. 'tilesHead' points to the head of the linked |
There was a problem hiding this comment.
box_mean (and also box, above) are no longer module variables, so it would be good to reflect this in the comments.
| ! | ||
| call get_box(latCell(iCell)*rad2deg, lonCell(iCell)*rad2deg, dc) | ||
| call get_box_size_from_lat_lon(latCell(iCell)*rad2deg, lonCell(iCell)*rad2deg, dc, nx, ny) |
There was a problem hiding this comment.
It looks like the cell coordinates are changed from degrees to radians inside the get_box_size_from_lat_lon routine. Would it be simpler to not convert from radians to degrees here and to also eliminate the reverse conversions inside the routine?
There was a problem hiding this comment.
I was trying to keep it in temporarily, so that it matches the previous logic and results. But removing it now.
| ! and the latitude and longitude coordinates (radians) | ||
| ! | ||
| !----------------------------------------------------------------------- | ||
| subroutine get_box_size_from_lat_lon(lat, lon, dx, nx, ny) |
There was a problem hiding this comment.
It doesn't look like lon is actually needed by this routine, which makes sense: the box size should be independent of longitude and should only depend on latitude.
| real (kind=RKIND), intent(inout) :: box_mean ! Mean value of topography in box | ||
|
|
||
| type(mpas_gwd_tile_type), pointer :: thisTile | ||
| integer :: i, j, ii, jj, ic, jc, ix, jx, tile_offset |
There was a problem hiding this comment.
It doesn't look like tile_offset is ever used.
|
|
||
| integer, parameter :: tile_x = 1200 ! x-dimension of each tile of global 30-arc-second landuse | ||
| integer, parameter :: tile_y = 1200 ! y-dimension of each tile of global 30-arc-second landuse | ||
|
|
||
| integer (c_int) :: istatus | ||
| integer :: ix, iy |
There was a problem hiding this comment.
With the changes in this PR, ix and iy are no longer used in this function.
|
|
||
| implicit none | ||
|
|
||
| character(len=*), intent(in) :: path | ||
|
|
||
| character(len=*), intent(in) :: sub_path |
There was a problem hiding this comment.
It doesn't look like sub_path is used.
| end function get_tile_from_box_point | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
I'd suggest only two blank lines here for consistency.
|
Thanks for the additional review @mgduda! I have addressed with the last two commits. |
A copy of #1228 with branch renamed. 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.