E3SM icon indicating copy to clipboard operation
E3SM copied to clipboard

hommexx limiter has extra barriers

Open oksanaguba opened this issue 1 year ago • 1 comments

the issue came up on weaver where the second barrier in this commit https://github.com/E3SM-Project/E3SM/pull/5328/commits/e391028308db2fcb744e9db353d24dd46814b66e (i put back ticks on purpose, so that comments for that commit are visible) was making the code (unit test only) hang, because one of columns triggers if-statement min_diff<0, but not the rest. i probably misunderstood the suggestion when i put these barriers in originally. the barriers are in || gll loop for column calculations being independent of each other. do we need barriers there?

@bartgol @ambrad

oksanaguba avatar Jul 04 '24 19:07 oksanaguba

The team_barrier is needed in principle, but it needs to be outside of the min_diff < 0 condition. I think this should work:

const bool min_diff_neg = min_diff < 0;
if (min_diff_neg) {
  // first chunk
}
kv.team_barrier();
if (min_diff_neg) {
  // second chunk
}

As a side note, placing team_barriers inside a TeamThreadRange is a bit questionable; probably we should rewrite any such case to have multiple TeamThreadRanges with the team_barriers between these.

ambrad avatar Jul 05 '24 17:07 ambrad