OpenMP using loops and array reductions

Question

I have written a program as follows:

#include "omp.h"
#include "stdio.h"

int main()
{
    int i, j, cnt[] = {0,0,0,0};
    #pragma omp parallel
    {
        int cnt_private[] = {0,0,0,0};
        #pragma omp for private(j)
        for(int i = 1 ; i <= 10 ; i++) {
            for(j = 1 ; j <= 10 ; j++) {
                int l= omp_get_thread_num();
                cnt_private[l]++;         
            }
            #pragma omp critical
            {   
               for(int m=0; m<3; m++){
                   cnt[m] = cnt_private[m];
               }
            }
           printf("%d %d %d %d %d\n",i,cnt[0],cnt[1],cnt[2],cnt[3]);
        }
     }
     return 0;
}

It should print the number of times each thread is executed for each i. As only one thread takes a particular i, the expected output should satisfy the sum of each row as 100. But I am getting the output of the form:

Where is the problem? Could it be in my fundamental understanding of OpenMP? or is my reduction process wrong? (I use a GNU gcc compiler and a 4 core machine) Compilation steps:

g++ -fopenmp BlaBla.cpp
export OMP_NUM_THREADS=4
./a.out

Each thread gets its own private cnt_private, so it will only ever have entries from its own thread id; and cnt just has a copy of cnt_private's data in it - from which ever thread is running the current i index. So what you have (modulo stopping the m loop too early) is correct. In particular, you are not doing a reduction on cnt. — Jonathan Dursi
– Jonathan Dursi, Commented Jun 23, 2015 at 19:11

MisterC · Accepted Answer · 2015-06-23 18:05:35Z

1

I do not see why the sum of each row should be 100.

You declared cnt_private to be private:

#pragma omp parallel
{
    int cnt_private[] = {0,0,0,0};
    // ...
}

As such the summation stored to it is not shared between threads. If thread l is executed only cnt_private[l] will be incremented and all others will be left at zero. Then you assing the content of cnt_private to cnt, which is not private. You assign every entry that is zero as well!

#pragma omp critical
{   
    for(int m=0; m<4; m++){ // I guess you want 'm<4' for the number of threads
        cnt[m] = cnt_private[m];
    }
}

With i ranging from 0 to 10 and the program using 4 threads, each threads gets 2 to 3 i's. As such I would expect the sum of each column to be either 30(10+20) or 60(10+20+30).

edited Jun 23, 2015 at 18:05

answered Jun 23, 2015 at 17:58

MisterC

2491 silver badge5 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

deba123 Over a year ago

Yeah I get it now, I had a typo on the m loop...thanks.

deba123 Over a year ago

So, the reduction of the array counter is not done accurately? Do I have to make 'cnt' private as well in order to get the expected result?

MisterC Over a year ago

I don't know what result you expect, could you elaborate on what the result should look like? Maybe you only want to assign cnt[l] = cnt_private[l]; or remove cnt_private and use cnt instead.

deba123 Over a year ago

I just want to know if reduction on the array was done correctly as we cannot directly use reduction clause on an array variable in C?

MisterC Over a year ago

The reduction is done correct, with one small mistake. In the line cnt[m] = cnt_private[m]; the current thread overrides values calculated by other threads. This is normally not wanted. You normally want cnt[m] += cnt_private[m];.

Collectives™ on Stack Overflow

OpenMP using loops and array reductions

1 Answer 1

5 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

5 Comments

Your Answer

Sign up or log in

Post as a guest

Related