How to make this code thread safe with openMP?

Question

The schematic code shown below works fine if I remove the #pragma omp parallel for, but with this in place the code compiles but then upon executing the binary I get errors like *** glibc detected *** ./testBin: double free or corruption (!prev): 0x0c43d8d8 *** and core dumped. I'm guessing that the reason is that multiple threads try to write to the variables omega, ell, .... or lineVec. How do I fix this? Is there a way to tell it the variables are shared? or is there just generally another way to do this loop in parallel. I am completely new to `openmp, this is the first time I have used it.

#include <omp.h>


int main( int argc , char **argv )
{ 
 vector <vector<string>> fileVec;
 //some code that reads in a CSV file lines into elements of fileVec

//variables constituting a line:
//my_float has been typedef to be a high precision class in real code
my_float omega;
my_float ell;

my_float init1Real;
my_float init1Imag;
my_float dinit1Real;
my_float dinit1Imag;

my_float init2Real;
my_float init2Imag;
my_float dinit2Real;
my_float dinit2Imag;

#pragma omp parallel for private(lineVec,fileVec,ell,omega,init1Real,init1Imag,dinit1Real,dinit1Imag,init2Real,init2Imag,dinit2Real,dinit2Imag)
 for (size_t i=0; i< fileVec.size(); i++) 
    { 

        lineVec=fileVec[i];


         ell=lineVec[0];
         omega=lineVec[1];

         init1Real=lineVec[2];
         init1Imag=lineVec[3];
         dinit1Real=lineVec[4];
         dinit1Imag=lineVec[5];
         init2Real=lineVec[6];
         init2Imag=lineVec[7];
         dinit2Real=lineVec[8];
         dinit2Imag=lineVec[9];


        // cout<<"OUTPUT ell=" << ell<< " omega=" << omega <<" init1Real="<<init1Real<<endl;

         //do some other calc involving these variables

    }     
   }

Hristo Iliev · Accepted Answer · 2012-12-06 09:37:54Z

5

Reading from a shared fileVec is thread-safe. Only the variables of type my_float should be made private or even better - declared inside the loop:

int main(int argc, char **argv)
{
    vector<vector<string>> fileVec;

    //some code that reads in a CSV file lines into elements of fileVec

    #pragma omp parallel for private(lineVec)
    for (size_t i = 0; i < fileVec.size(); i++)
    {
        lineVec = fileVec[i];

        //my_float has been typedef to be a high precision class in real code

        my_float ell = lineVec[0];
        my_float omega = lineVec[1];

        my_float init1Real = lineVec[2];
        my_float init1Imag = lineVec[3];
        my_float dinit1Real = lineVec[4];
        my_float dinit1Imag = lineVec[5];
        my_float init2Real = lineVec[6];
        my_float init2Imag = lineVec[7];
        my_float dinit2Real = lineVec[8];
        my_float dinit2Imag = lineVec[9];

        cout << "OUTPUT ell=" << ell << " omega=" << omega
             << " init1Real=" << init1Real << endl;

        //do some other calc involving these variables
    }
}

I don't see any races here unless my_float is not thread-safe or there is something else hidden in the //do some other calc involving these variables.

Note that with recent OpenMP versions you can even use iterators to walk the vector, since it provides a random access iterator:

typedef vector<vector<string>>::const_iterator iterType;

#pragma omp parallel for private(lineVec)
for (iterType it = lineVec.begin(); it != lineVec.end(); it++)
{
    ...
}

answered Dec 6, 2012 at 9:37

Hristo Iliev

74.8k12 gold badges148 silver badges197 bronze badges

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

6 Comments

fpghost Over a year ago

OK, that seems to work (except the strange cout behaviour of course). Is it really better to declare ell,omega,.. etc inside the loop rather than make them private? either way seems to work in terms of no core dumps, but just wondering which is most efficient.

fpghost Over a year ago

as for the //do some other calc involving these variables. One thing I do after the calculation for each omega, ell is write the results out to a file; the file is different for each loop (each omega,ell so this shouldn't cause any problems, right?

Hristo Iliev Over a year ago

Variables declared inside a parallel region are automatically private. Also since parallel regions are usually extracted into separate functions, these variables won't take stack space in the parent function. And also in C++ and C99 it is a good programming practice to only declare variables in the scope where they are being used.

Hristo Iliev Over a year ago

Writing to a separate file in each iteration should not be a problem. By the way, if you do not use lineVec outside of the loop, you can declare it inside and have the privte(lineVec) clause removed.

fpghost Over a year ago

thanks, all seems to be working well now. Is it necessary for me to do things like set the number of cores etc or will this be taken care of automatically to use the max? anything else I need to do to improve execution speed? thanks again.

|

Étienne · Accepted Answer · 2012-12-06 09:57:12Z

1

the way you wrote it, openmp will create some threads and divide the total number of iterations of the for loop between every threads. By doing so it will try to perform parallel reads on the vector shared by the different threads. You can change the data sharing attributes, (see the OpenMP Wiki about data sharing attribute clauses, and in this microsoft doc you have a good example how to do it. For an example, to declare lineVec and fileVec as "private" use:

#pragma omp parallel private(lineVec, fileVec)

Moreover, cout is not thread safe and calls to cout from the multiple threads also need to be serialized.

edited Dec 6, 2012 at 9:57

answered Dec 5, 2012 at 22:28

Étienne

5,0773 gold badges39 silver badges67 bronze badges

4 Comments

fpghost Over a year ago

thanks, but even doing

#pragma omp parallel for private(lineVec,fileVec,ell,omega,init1Real,init1Imag,dinit1Real,dinit1Imag,init2Real,init2Imag,dinit2Real,dinit2Imag)

and removing the cout hasn't stopped the segmentation fault (core dumped) errors at runtime. I should mention that fileVec is very large (it is the whole CSV file read in) so if this private makes lots of copies of it, maybe running out of memory could be an issue. I changed the code in the OP to what I have with your modifications.

fpghost Over a year ago

Looking at the wiki article would I not want fileVec to be firstprivate to it gets initialized to its original value that I have set by reading in the CSV higher up in the code? and only omega, ell, init1Real... that I set during the for to be private?

fpghost Over a year ago

the size of fileVec is 67881, so not a multiple of 10. But the size of each of fileVec 's elements that set lineVec is always 10. In the real code I have an if (lineVec.size() == 10) wrapped around where I set omega,ell,... etc in the for loop.

Hristo Iliev Over a year ago

The note about reading from a shared vector not being thread-safe is pure nonsense, unless the vector is being modified by some of the threads (which is clearly not the case here).

Collectives™ on Stack Overflow

How to make this code thread safe with openMP?

2 Answers 2

6 Comments

4 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

6 Comments

4 Comments

Your Answer

Sign up or log in

Post as a guest

Related