1

I am new to openmp, when I add openmp into my code, I found the results are not the same in different run. is this the inherent problem of openmp or my code problem? Thank you!

#include "stdafx.h"
#include <iostream>
#include <vector>
#include <fstream>
#include <math.h>
#include<sstream>
#include <omp.h>
using namespace std;

int main()
{
    double a[1000];
    for (int i = 0; i < 1000; i++)
    {
        a[i] = 0;
    }
    for (int m = 0; m < 1000; m++)
    {
    #pragma omp parallel for shared(a)
        for (int i = 0; i < 1000000; i++)
        {
            int b = pow(i, 0.5);
            if (b < 1000)
            {
                //cout << i <<" "<<sin(i)<< endl;

                a[b] += sin(i);
            }
        }
    }

    fstream all_temp;
    all_temp.open("temperatureabcd.dat", fstream::out);
    for (int aaa = 0; aaa < 1000; aaa++)
    {
        all_temp << a[aaa] << endl;
    }
    all_temp.close();
    return 0;
}
Z boson
  • 32,619
  • 11
  • 123
  • 226
Lun Li
  • 13
  • 2
  • Two problems. You are doing an array reduction. You need `#pragma omp parallel for reduction(+:a[1000])` but you're using MSVC which does not support array reductions. Get a better compiler (GCC, Clang, or ICC) or do the reduction by hand. Second problem: floating point arithmetic is not associative so the order you add numbers matters. But the array reduction is your major problem. – Z boson Aug 03 '18 at 09:33

1 Answers1

1

Your code is doing an array reduction. The simple solution would be to do

#pragma omp parallel for reduction(+:a[:1000])

However MSVC which you are using (which I infer from the precompiled header stdafx.h) does not support OpenMP array reduction. You can do the array reduction by hand by changing the code a bit like this

double a[1000] = {0};
for (int m = 0; m < 1000; m++) {
  #pragma omp parallel
  {
    double a2[1000] = {0};
    #pragma omp for nowait
    for (int i = 0; i < 1000000; i++) {
      int b = pow(i, 0.5);
      if (b < 1000) a2[b] += sin(i);
    }
    #pragma omp critical
    for(int i = 0; i < 1000; i++) a[i] += a2[i];
  }
}

The other problem is that floating point addition is not associative so the order that the reduction is done matters. This can be fixed with a bit more work but it's probably not your main problem.

Z boson
  • 32,619
  • 11
  • 123
  • 226
  • appreciate your help! Would you tell me what compiler do you use rather than MSVC? Since my code has only one file, it is easy to transplant to other compiler. my code is a little bit sensitive about time consuming, I want to check whether it is more time efficient when using reduction in other compiler. thanks again1 – Lun Li Aug 03 '18 at 13:47
  • @LunLi, I mostly use GCC and ICC. GCC, ICC, Clang, and I think even PGI (best OpenACC compiler that also supports OpenMP) support array reductions. MSVC is the only one that does not. – Z boson Aug 03 '18 at 13:56
  • appreciate your help again. I am sorry to bother you again. Actually, the array is a dynamic opened array in my code. If I use reduction, should I use reduction(+:a[n])? I integrated Intel C++ compiler into visual studio. Does it work the same as independent ICC compiler? Thanks – Lun Li Aug 03 '18 at 22:23
  • @LunLi, ICC should work the same in Visual Studio. I think I had my notation wrong. It is `reduction(+:a[:1000])`. In fact I think you can just do `reduction(+:a)`. But I think `a` has to be an array and not a pointer. I'm not really sure. See https://stackoverflow.com/questions/20413995/reducing-on-array-in-openmp/20421200#comment62676230_20413995 – Z boson Aug 06 '18 at 07:47
  • Appreciate your help. my code is paralleled now, but it is even slower than the none parallel one. I think it may be that the loop part is not large enough. – Lun Li Aug 06 '18 at 19:02
  • @LunLi. It should not be any slower. There is probably something else wrong. The work sharing loop in your question has a range of 1000000. That should be large enough to dominate the OpenMP overhead. BTW, you will likely get faster results using `-Ofast`. – Z boson Aug 07 '18 at 07:02
  • Appreciate! I will check it. – Lun Li Aug 07 '18 at 21:32