0

I am quite new to OpenMP. I have the following simple loop that I want to run in parallel with OpenMP:

double rij[3];
double r;

#ifdef _OPENMP
#pragma omp parallel for private(rij,r)
#endif
for (int i=0; i<n; ++i)
{
    for (int j=0; j<n; ++j)
    {
        if (i != j)
        {
            distance(X,rij,r,i,j);

            V[i] += ke * Q[j] / r;

            for (int k=0; k<3; ++k)
            {
                F[3*i+k] += ke * Q[j] * rij[k] / pow(r,3);
            }
        }
    }
}

From what I understood, variables are shared by default which is why I only declared private(rij,r). But according to these questions (first second third), I should do array reduction in this case.

It's clear to me that if many threads need to sum to the same variable, this has to be done with #pragma omp parallel for reduction(+:A[:n]) for summing to array A of size n. This is what I do in another part of my code, and it works as expected.

However, in this case workers never have to sum to the same variable: every worker performs the sum on its index i. Is is correct to do as I do in this case i.e. not doing any array reduction and not using any critical section ?

If my implementation is correct, I believe it would avoid the overhead of the critical section while being simpler code. Feel free to give your advice on how this could be better optimized.

Thank you

Toool
  • 361
  • 3
  • 18
  • This code is broken. The value of "r" and "rij" is undefined inside the parallel region. I think you want those shared (unless distance is writing them via references; in which case declare them in the right scope!). – Jim Cownie Jul 10 '20 at 07:55
  • Yes sorry, the code was not meant to be working out of the box, but just to illustrate my point. The `distance` function writes to `r` and `rij` as a function of `i` and `j`, so I want them to be private. – Toool Jul 11 '20 at 10:07
  • Then declare them in their minimal scope, to make that clear and so you don't have to add directives which you can get wrong. It's not as if declaring variables in inner scopes is a new feature! – Jim Cownie Jul 13 '20 at 07:45
  • You must make `j` and `k` private as well in the `parallel for` directive as they're shared by default. (The outer loop index `i` is private by default). – user1551605 Apr 13 '23 at 16:01

1 Answers1

1

You don't need a reduction. It is a feature to avoid copying the same code all over again because they are re-occurring problems (Try to think off, how you would implement a sum-reduction without OpenMP).

What you do right now is working on parallel data (V[i]) which should not overlap at any iteration (as you state in the question), because you divide by i itself. Furthermore write to F[...] shouldn't overlap either, because it only depends on iand k

RoQuOTriX
  • 2,871
  • 14
  • 25