Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hommexx limiter has extra barriers #6500

Open
oksanaguba opened this issue Jul 4, 2024 · 1 comment
Open

hommexx limiter has extra barriers #6500

oksanaguba opened this issue Jul 4, 2024 · 1 comment
Assignees

Comments

@oksanaguba
Copy link
Contributor

oksanaguba commented Jul 4, 2024

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 oksanaguba self-assigned this Jul 4, 2024
@ambrad
Copy link
Member

ambrad commented Jul 5, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants