Multi threading program to count the frequency of prime numbers giving inaccurate results.

Question

I am working on a assignment where I need to calculate the frequency of prime numbers from 1 to 10 million. we are to do this by taking variable U (which is 10 million) and dividing it into N parts and have multiple threads calculate the frequency of prime numbers, we must try this with different values for N and observe our processor ticks and time taken to calculate. The way the program works is 10 million is divided into N parts, upper and lower bounds are put into a vector of threads and each thread calls a function which counts the prime numbers. now the frequency of primes from 1-million should be 664579. i am getting slightly inaccurate results when doing multiple threads. for example if i run the program with N=1, meaning only one thread will solve i get a frequency of 6645780, which is off by 1. N=2 i get the correct result of 664579, N=3 freq=664578, and so on. below is the code, any help is greatly appreciated.

#include <iostream>
#include <thread>
#include <vector>
#include<algorithm>
#define MILLION 1000000
using namespace std;
using std::for_each;
void frequencyOfPrimes(long long upper, long long lower, long long* freq)
{
    long long i, j;
    if (lower == 2) { *freq = upper; }
    else { *freq = upper - lower; }
    for (i = lower; i <= upper; ++i)
        for (j = (long long)sqrt(i); j>1; --j)
            if (i%j == 0) { --(*freq); break; }
    return;
}

int main(int argc, char* argv[])
{

    clock_t ticks = clock();
    long long N = 10; //declare and initialize number of threads to calculate primes
    long long U = 10*MILLION;
    long long F=0; //total frequency
    long long d = U / N; // the quotient of 10mil/number of threads
    vector<thread> tV; //declare thread vector
    vector<long long> f, u, l; //vector for freq, upper and lower bounds
    f.resize(N);//initialize f
    for (long long i = 0; i<N; i++) { //initialize and populate vectors for upper and lower bounds
        if (i == 0) {
            l.push_back(2);
            u.push_back(d);
        }
        else {
            l.push_back(u.at(i-1)+ 1);
            u.push_back(u.at(i-1) + d);
        }
    }
    u.at(N-1) = U;  //make sure last thread has value of U for upper bound
    for (long long i = 0; i < N; i++) { //initialize thread vectors
        tV.push_back(thread(frequencyOfPrimes, u.at(i), l.at(i), &f.at(i)));
    }

    for_each(tV.begin(), tV.end(), mem_fn(&thread::join));

    ticks = clock() - ticks;

    for (long long i = 0; i < N; i++)
        F = f.at(i) + F;
    cout << "Frequency is " << F << endl;
    cout << "It took " << ticks << " ticks (";
    cout << ((float)ticks) / CLOCKS_PER_SEC << " seconds)" << endl;
    this_thread::sleep_for(chrono::seconds(5));
    return 0;
}

Answer 1

This has nothing to do with multi threading. Always test your functions:

#include <iostream>
#include <cmath>
using namespace std;

// this is your function with a shorter name
void fop_strange(long long upper, long long lower, long long* freq)
{
    long long i, j;
    if (lower == 2) { *freq = upper; }
    else { *freq = upper - lower; }
    for (i = lower; i <= upper; ++i)
        for (j = (long long)sqrt(i); j>1; --j)
            if (i%j == 0) { --(*freq); break; }
    return;
}

// attention, I switched order of upper and lower
long long fop(long long a, long long b) {
  long long f = 0;
  fop_strange (b, a, &f);
  return f;
}

int main() {
  cout << fop(2, 4) << endl;
  cout << fop(10, 14) << endl;
  return 0;
}

Let's first count the primes manually:

2 to 4 (inclusive) => 2 (2, 3)
10 to 14 (inclusive) => 2 (11, 13)

Now your function (live on ideone)

3
1

Why? Well, you're correctly decreasing the count when you encounter a non prime, but you don't initialise it correctly. The range from 2 to 4 inclusive has 3 numbers, not 4. The range from 2 to 1 million has 1 million - 2 + 1, not 1 million numbers. The range from 10 to 14 inclusive has 5 numbers, not only 4. Etc.

This explains the results you're getting: For a range that starts from 2 your function returns 1 number more, fit every other range 1 number less. Therefore, when using only one thread and thus only a single range starting with 2 your result is one more, and every thread you add adds a range that brings one less, thus decreasing the overall result by one.