Yes, what you did is correct, and what it was before is wrong,
and, in fact, is a parallel bug if the code is compiled/used in
Open MP mode.
The explanation is as follows: if you examine
Code: Select all
./roms-3.1/ROMS/Functionals/ana_grid.h
You will find that shared arrays xr(i,j)=... and yr(i,j)=... are
assigned their values at about line 500 (just before CALL
mp_exchange2d take place) over the range of indices
where the ranges Imin,Imax,Jmin,Jmax are defined from tile ranges
Istr,Iend,Jstr,Jend by adding an extra point if next to the boundary.
Then assignment of h(i,j)=... in the SEAMOUNT code segment around
line 820 (the place you have modified) occurs over the ranges
Code: Select all
DO j=JstrR,JendR
DO i=IstrR,IendR
which are basically... the same ranges covering tile + extra row
of points next to the boundary.
If no tiling, then the original code would work correctly because
xr(Lm/2,Mm/2)=... and yr(Lm/2,Mm/2)=... are assigned their values
BEFORE they are used to compute h(i,j)=...
However, if number of tiles in either direction is more than one
then there is a chance that h(i,j)=... is attempts to use values of
xr(Lm/2,Mm/2) and yr(Lm/2,Mm/2) BEFORE THEIR VALUES WERE ASSIGNED.
This is because the order of processing has been changed, once
tiling is introduced: Now the subroutine "ana_grid_tile" assignes
BOTH xr,yr AND within only range of indices corresponding to a single
tile, then proceeds to the next tile, and so on, and if more tan one
CPU (or core) is used, then different tiles are given to different
CPUs, but not necessarily in one-to-one corespondence (i.e., it is
OK, and in fact, should be done to get better efficiency, that each
CPU processes several tiles).
Note that in Open MP code you should not assume that the neighboring
CPU has completed its job, unless there is a syncronization point:
OMP$ BARRIER or end of PARALLEL DO. For this reason A SHARED array
(in this case xr,yr, etc) assigned within a subroutine CANNOT be used
as an input, except in a strictly local fashion, e.g.,
Code: Select all
DO j=JstrR,JendR
DO i=IstrR,IendR
xr(i,j)=...
ENDDO
ENDDO
.....
DO j=JstrR,JendR
DO i=IstrR,IendR
something(i,j) = xr(i,j)
ENDDO
ENDDO
is OK, but
Code: Select all
DO j=JstrR,JendR
DO i=IstrR,IendR
xr(i,j)=...
ENDDO
ENDDO
.....
DO j=JstrR,JendR
DO i=IstrR,IendR
something(i,j) = xr(i,j)-x(i-1,j)
ENDDO
ENDDO
is NOT OK.
If array xr would be PRIVATE, then the following code
Code: Select all
DO j=JstrR,JendR
DO i=Istr-1,IendR
xr(i,j)=...
ENDDO
ENDDO
.....
DO j=JstrR,JendR
DO i=IstrR,IendR
something(i,j) = xr(i,j)-x(i-1,j)
ENDDO
ENDDO
is OK.
But the above is NOT OK, if xr is SRARED: using i-index range
DO i=Istr-1,IendR may cause race condition if two CPUs try to
assign values into the same memory location.