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

Fix modulo in GCL #1355

Merged
merged 1 commit into from
Oct 7, 2019
Merged

Fix modulo in GCL #1355

merged 1 commit into from
Oct 7, 2019

Conversation

lukasm91
Copy link
Contributor

@lukasm91 lukasm91 commented Sep 2, 2019

If m_coordines[x] == 0 and X == -1, we might do -1 % N. The sign of the result was implementation-defined pre-stdc++11; in GCC 7.3 this is incorrectly still negative, which leads to problems with MPI later in the code.

(Maybe), backport to gridtools 1.0 required.

@lukasm91
Copy link
Contributor Author

lukasm91 commented Sep 2, 2019

launch strgrid

@@ -181,23 +181,23 @@ namespace gridtools {
int _coords[3];

if (m_cyclic.value(0))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does m_cyclic mean?

Copy link
Contributor Author

@lukasm91 lukasm91 Sep 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whether it should be a cyclic communicator. If the communicator is not cyclic, we don't need to communicate; so we don't need the fix in the else case.

@havogt
Copy link
Contributor

havogt commented Sep 2, 2019

We need the backport. We can do it once this is reviewed.

@havogt
Copy link
Contributor

havogt commented Sep 2, 2019

launch mpistrgrid

@havogt havogt merged commit 2d3e74e into GridTools:master Oct 7, 2019
havogt pushed a commit that referenced this pull request Oct 7, 2019
If `m_coordines[x] == 0` and `X == -1`, we might do `-1 % N`. The sign of the result was implementation-defined pre-stdc++11; in GCC 7.3 this is incorrectly still negative, which leads to problems with MPI later in the code.

Backport of #1355
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

Successfully merging this pull request may close these issues.

3 participants