Parallelizing a for loop in Visual Studio 2010 (OpenMP)

Question

I've recently reading up about OpenMP and was trying to parallelize some existing for loops in my program to get a speed-up. However, for some reason I seem to be getting garbage data written to the file. What I mean by that is I don't have Points 1,2,3,4 etc. written to my file, I have Points 1,4,7,8 etc. I suspect this is because I am not keeping track of the threads and it just leads to race conditions?

I have been reading as much as I can find about OpenMP, since it seems like a great abstraction to do multi-threaded programing. I'd appreciate any pointers please to get to the bottom of what I might be doing incorrectly.

Here is what I have been trying to do so far (only the relevant bit of code):

#include <omp.h>

pixelIncrement = Image.rowinc/2;

#pragma omp parallel for
for (int i = 0; i < Image.nrows; i++ )
   {
      int k =0;
      row     = Image.data + i * pixelIncrement;

      #pragma omp parallel for
      for (int j = 0; j < Image.ncols; j++)
      {
          k++; 
          disparityThresholdValue = row[j];  
                 // Don't want to save certain points
             if ( disparityThresholdValue < threshHold)
             {       
                // Get the data points
                x = (int)Image.x[k]; 
                y = (int)Image.y[k]; 
                z = (int)Image.z[k]; 
                grayValue= (int)Image.gray[k];      

                cloudObject->points[k].x = x;
                cloudObject->points[k].y = y;
                cloudObject->points[k].z = z;
                cloudObject->points[k].grayValue = grayValue;

        fprintf( cloudPointsFile, "%f %f %f %d\n", x, y, z, grayValue);
             }
      }
   }

   fclose( pointFile );

I did enable OpenMP in my Compiler settings (C/C++ -> Language -> Open MP Support (/openmp).

Any suggestions as to what might be the problem? I am using a Quadcore processor on Windows XP 32-bit.

Answer 1

Are all points written to the file, but just not sequentially, or is the actual point data messed up?

The first case is expected in parallel programming - once you execute something side-by-side you wont be able to guarantee order unless you synchronize the access (at which point you can just leave out the parallelization as it becomes effectively linear). If you need to rely on order, you can parallelize any calculations but need to write it down in one thread.

If the points itself are messed up, check where your variables are declared and if multiple threads are accessing the same.

Answer 2

A few problems here:

#pragma omp parallel for
for (int i = 0; i < Image.nrows; i++ )
   {
      int k =0;
      row     = Image.data + i * pixelIncrement;

      #pragma omp parallel for
      for (int j = 0; j < Image.ncols; j++)
      {
          k++; 

There's no need for the inner parallel for. The outer loop should contain enough work to keep all cores busy.

Also, for the inner loop k is a shared variable and gets incremented in a non-atomic way. x, y, z are also shared among the inner loop threads and overwritten "randomly". Remove the inner directive and see how it goes.