patterncMinor
OpenMP parallelization of a for loop with function calls
Viewed 0 times
callswithloopfunctionopenmpparallelizationfor
Problem
Using OpenMP, is it correct to parallelize a
Where:
I know that, for a correctness, a special treatment of function
but apart from that, are there some other important (conceptual) correction to do on the above parallelization? Which is limited at only one directive row?
for loop inside a function "func" as follows?void func(REAL coeff, DATAMPOT *dmp, int a, int la, int b, int lb, REAL L)
{
int i,j,k;
REAL dx,dy,dz;
REAL dx2,dy2,dz2;
REAL r;
#pragma omp parallel for default(shared) private(k,i,j,dx,dy,dz,dx2,dy2,dz2,r) reduction(+:deltaE)
for(k=0; knpot>1?
mpot(r,dmp+NSPES*part[i].s+part[j].s,((REAL)rand())/RAND_MAX):
(dmp+NSPES*part[i].s+part[j].s)->pot[0](r,(dmp+NSPES*part[i].s+part[j].s)->dp ) );
}
}Where:
REALisdouble(#define REAL double)
DATAMPOT *dmpis a pointer to a struct containing (among others) some pointers to functions, such aspot[0]
partis a global array ofstruct
deltaE(variable for summation-reduction) is aREALglobal variable
I know that, for a correctness, a special treatment of function
rand() is also required;but apart from that, are there some other important (conceptual) correction to do on the above parallelization? Which is limited at only one directive row?
Solution
-
Try to come up with a better function name than
-
Try not to use single-character names, unless they're for a simple
-
It's a little hard to read lines lacking whitespace:
It's difficult to see the entire ternary statement, so add more whitespace:
This concept should be applied everywhere, especially each statement in the loop.
Going back to the ternary, consider using an
-
This formatting is a bit hard to read:
At least reformat it to something like this:
However, this may still be too lengthy for a ternary. You could still use a plain
Try to come up with a better function name than
func(). You are the one who understands this code the most, so you should know how to give it a relevant name.-
Try not to use single-character names, unless they're for a simple
for-loop. It's hard to tell what they're for, especially without comments. If this ends up making the #pragma line even longer, then you can just split it into separate lines with a \ at the end of each line.-
It's a little hard to read lines lacking whitespace:
dx2=(dx<0.5?dx*dx:(1-dx)*(1-dx));It's difficult to see the entire ternary statement, so add more whitespace:
dx2 = (dx < 0.5 ? dx*dx : (1-dx)*(1-dx));This concept should be applied everywhere, especially each statement in the loop.
Going back to the ternary, consider using an
if/else here instead. Sure, this is a short ternary, but that doesn't mean it should always be used, especially if it's harder to read carefully.if (dx < 0.5)
{
dx2 = dx * dx;
}
else
{
dx2 = (1-dx) * (1-dx);
}-
This formatting is a bit hard to read:
deltaE += coeff*((dmp+NSPES*part[i].s+part[j].s)->npot>1?
mpot(r,dmp+NSPES*part[i].s+part[j].s,((REAL)rand())/RAND_MAX):
(dmp+NSPES*part[i].s+part[j].s)->pot[0](r,(dmp+NSPES*part[i].s+part[j].s)->dp ) );At least reformat it to something like this:
deltaE += coeff*((dmp+NSPES*part[i].s+part[j].s)->npot>1
? mpot(r,dmp+NSPES*part[i].s+part[j].s,((REAL)rand())/RAND_MAX)
: (dmp+NSPES*part[i].s+part[j].s)->pot[0](r,(dmp+NSPES*part[i].s+part[j].s)->dp));However, this may still be too lengthy for a ternary. You could still use a plain
if/else if it would help more with readability.Code Snippets
dx2=(dx<0.5?dx*dx:(1-dx)*(1-dx));dx2 = (dx < 0.5 ? dx*dx : (1-dx)*(1-dx));if (dx < 0.5)
{
dx2 = dx * dx;
}
else
{
dx2 = (1-dx) * (1-dx);
}deltaE += coeff*((dmp+NSPES*part[i].s+part[j].s)->npot>1?
mpot(r,dmp+NSPES*part[i].s+part[j].s,((REAL)rand())/RAND_MAX):
(dmp+NSPES*part[i].s+part[j].s)->pot[0](r,(dmp+NSPES*part[i].s+part[j].s)->dp ) );deltaE += coeff*((dmp+NSPES*part[i].s+part[j].s)->npot>1
? mpot(r,dmp+NSPES*part[i].s+part[j].s,((REAL)rand())/RAND_MAX)
: (dmp+NSPES*part[i].s+part[j].s)->pot[0](r,(dmp+NSPES*part[i].s+part[j].s)->dp));Context
StackExchange Code Review Q#26515, answer score: 5
Revisions (0)
No revisions yet.