Programowanie współbieżne, suma szeregu

Witam, mam problem ponieważ napisałem taki program który ma liczyć sumę szeregu przy użyciu dowolnej liczby wątków jednakże mój program działa niepoprawnie ponieważ wynik mnoży przez ilość wątków., Prosze o pomoc w tej sprawie.(program ma być napisany bez użycia struktur)

#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
#include <pthread.h>

#define MAXL 100000000
#define LICZBAW 2


double suma_tab[LICZBAW];
double start = 0;
double koniec = MAXL/LICZBAW; 


double suma(int p,int k) {
     double s=0;
     int i;
     for (i=p;i<=k;i++) s=s+(1.0/i)*(1.0/i);
     return s;
}

void* p (void* l) {
    long n = (long)l;
	 double s;
     
     s=suma(start+1,koniec);
     start = koniec;
     koniec += koniec;
     
     suma_tab[n] += s;
     return 0;     
}


int main () {

	pthread_t w[LICZBAW];
	long i;

	
	for(i=0; i<LICZBAW; i++){
		pthread_create(&w[i], 0, p, (void*)i);
	
	}
	for(i=0; i<LICZBAW; i++)
		{
			pthread_join(w[i], NULL);
		}
		
	double sum = 0;
    for ( i = 0; i < LICZBAW; i++) {
        sum += suma_tab[i];
    }

    printf("Suma: %.15lf\n", sum);
	
	return 0;
}

Proponuję najpierw uruchamiać funkcję p bez wątku. Nie powinno mieć to żadnego znaczenia, czy kilka instancji funkcjo jest uruchamianych jednocześnie, tak, jak teraz masz, czy funkcja jest wielokrotnie uruchamiana bez wątków (czyli w praktyce kolejne instancje są jedna po drugiej).

Jak przerobisz program na taki bez użycia wątków, to po prostu uruchomi funkcję p tyle razy, ile wskazuje LICZBAW. Jeśli chodzi o argumenty dla poszczególnych wywołań funkcji, to też nie powinno mieć znaczenia, czy sekwencja jest od przodu czy od tyłu.

W ten sposób (czyli bez wątków) będzie dużo łatwiej debugować i obserwować przebieg obliczeń i tym samym łatwiej będzie znaleźć przyczynę błędnych obliczeń.

Potem możesz odwrócić kolejność wywołań funkcji p (właściwie to wielokrotnie uruchamiasz tą samą funkcję, ale chodzi o kolejność zestawów argumentów). Wynik obliczeń powinien być taki sam. Dopiero dodaj wątki, czyli tak, jak teraz masz.

Jeśli dobrze rozumiem, to przy dwóch wątkach, pierwszy wątek obliczy pierwszą połowę, drugi obliczy drugą połowę, a następnie sumujesz sumy częściowe. Czy faktycznie taki jest pomysł?

Jednakże ustawienia zmiennych globalnych start i koniec z wątku może być przyczyną problemu. Jeżeli to są numery pierwszego i ostatniego elementu, które mają być sumowane w wątku, to te liczby lepiej przekazać przez parametry zamiast globalnych. Ewentualnie zamieniasz na tablice, która ma tyle elementów, ile ma wątków, wypełniasz ją i wtedy uruchamiasz wątek, przekazując do każdego numer kolejny od 0, inny dla każdego wątku.

Przy wielu wątkach, faktyczny moment rozpoczęcia i zakończenia każdego wątku nie może mieć znaczenia. Czasami może się zdarzyć, że kolejność uruchamiania wątków nie będzie równa kolejności ich tworzenia, jeżeli są tworzone w bardzo krótkim odstępie czasu.

Przecież ten kod ma ewidentny race condition na start i koniec.
Uruchomianie w jednym wątku ukryje ten problem.
Zmień kod tak, by te wartości nie były zmiennymi globalnymi.

Nawet thread sanitizer ładnie to znajduje: https://godbolt.org/z/96cf7aKao

Nie wiem jaki jest twój skill level (kod wskazuje, na to, że przekroczyłeś już level newbie), ale wielowątkowość jest trudna. Największy problem to fakt, że błędy dają niedeterministyczne rezultaty (niepoprawny kod może sprawiać wrażenie, że działa).
Jeśli nadal jesteś początkujący to lepiej sobie daruj, lepiej spożytkujesz czas nauki poświęcając go innym tematom, np: pisanie testów. Nawet na średnim poziomie umiejętności trzeba uważać jakie funkcjonalności wielowątkowości się używa.

Nie wiem w sumie, co chcesz zrobić, ale problem jest w funkcji p, jak napisał MarekR22. Jeśli chcesz to zrobić bez locków, to najpierw podziel sobie zakres [start, koniec] na tyle odcinków co masz wątków. W pętli pthread_create odpalaj z argumentami wskazującymi na początek i koniec wybranego odcinka, to nie będziesz miał race condition. Jak nic nie chcesz synchronizować, to możesz np. zebrać wyniki w tablicy, której k-ty element to wynik działania k-tego wątku. Jak tak to zrobisz, to nie będziesz miał race condition.

Aha - jest tam pewna złośliwość: skoro program ma być napisany bez struktur, to musisz pomyśleć, jak przekazać parę początek/koniec do wątku. Ale to nie powinien być problem. Swoją drogą: dlaczego bez struktur?

Nie jestem dobry w C, ale tu jest moja wersja: https://godbolt.org/z/1qooT6ssW

Trudno wyczytać jaki szereg tu sumujesz, ale jeśli dobrze zrozumiałem to jest to

X = sum(i=1..MAXL; (1.0/i)*(1.0/i))

Kod przy użyciu OpenMP:

#include <stdio.h>
#include <omp.h>


#define MAXL 100000000

int main(int argc, char** argv){
    double partial_Sum, total_Sum;

    #pragma omp parallel private(partial_Sum) shared(total_Sum)
    {
        partial_Sum = 0;
        total_Sum = 0;

        #pragma omp for
        for(int i = 1; i <= MAXL; i++){
            partial_Sum += (1.0/i)*(1.0/i);
        }
        
        #pragma omp critical
        {
                //add each threads partial sum to the total sum
                total_Sum += partial_Sum;
        }        
    }
    
    printf("total_sum: %f\n", total_Sum);
    return 0;
}

Uruchomienie:

g++ testsum.c -o testsum -fopenmp

OMP_NUM_THREADS=6;
export OMP_NUM_THREADS
echo test with ${OMP_NUM_THREADS} threads
time ./testsum

echo
OMP_NUM_THREADS=4;
export OMP_NUM_THREADS
echo test with ${OMP_NUM_THREADS} threads

time ./testsum
echo

OMP_NUM_THREADS=2;
export OMP_NUM_THREADS
echo test with ${OMP_NUM_THREADS} threads

time ./testsum

Wynik:

test with 6 threads
total_sum: 1.644934

real	0m0,224s
user	0m1,148s
sys	0m0,000s

test with 4 threads
total_sum: 1.644934

real	0m0,198s
user	0m0,791s
sys	0m0,000s

test with 2 threads
total_sum: 1.644934

real	0m0,403s
user	0m0,805s
sys	0m0,000s

Więcej o OpenMP (na tym bazowałem): https://curc.readthedocs.io/en/latest/programming/OpenMP-C.html#work-sharing-directive-omp-for

Z drobnymi poprawkami, już bez data raceów:

#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

#define MAXL 100000000.0
#define LICZBAW 2

double suma_tab[LICZBAW];
_Atomic int start = 0;
int chunk_len  = MAXL / LICZBAW;

double suma(int p, int k) {
  double s = 0;
  int i;
  for (i = p; i <= k; i++)
    s = s + (1.0 / i) * (1.0 / i);
  return s;
}

void *p(void *l) {
  long n = (long)l;
  double s;

  int koniec = (start += chunk_len);
  s = suma(koniec - chunk_len + 1, koniec);

  suma_tab[n] += s;
  return 0;
}

int main() {

  pthread_t w[LICZBAW];
  long i;

  for (i = 0; i < LICZBAW; i++) {
    pthread_create(&w[i], 0, p, (void *)i);
  }
  for (i = 0; i < LICZBAW; i++) {
    pthread_join(w[i], NULL);
  }

  double sum = 0;
  for (i = 0; i < LICZBAW; i++) {
    sum += suma_tab[i];
  }

  printf("Suma: %.15lf\n", sum);

  return 0;
}

Wielkie dzięki za poświęcony czas i pomoc. Finalnie udało się coś wklepać i naprawić

Wydaje mi się że da się to załatwić bardziej elegancko, ba bez zewnętrznych bibliotek:

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

class ChunkSummator
{
	private:
	long double sum=0;
	long long count,start;
	int chunkcount;
	thread th;
	long double func(long long i) { return 1/(i*(long double)i); }
	void sumfunc() { for(long long i=start+1;i<=count;i+=chunkcount) sum+=func(i); }
	static void call(ChunkSummator *cs) { cs->sumfunc(); }
	public:
	void calculate(long long count,long long start,int chunkcount=8)
	{
		this->count=count;
		this->start=start;
		this->chunkcount=chunkcount;
		this->sum=0;
		th=thread(call,this);
	}
	long double join()
	{
		th.join(); 
		return sum;
	}
};


int main()
{
	constexpr int chunkcount=8;
	ChunkSummator ths[chunkcount];
	long double sum=0;
	for(int c=0;c<chunkcount;++c) ths[c].calculate(10,c,chunkcount);
	for(int c=0;c<chunkcount;++c) sum+=ths[c].join();
	cout<<sum<<endl;
	return 0;
}

Lub:

#include <iostream>
#include <vector>
#include <thread>
using namespace std;

class ChunkSummator
{
	private:
	long double sum;
	long long count,start;
	int chunkcount;
	thread th;
	long double func(long long i) { return 1/(i*(long double)i); }
	void sumfunc() { for(long long i=start+1;i<=count;i+=chunkcount) sum+=func(i); }
	static void call(ChunkSummator *cs) { cs->sumfunc(); }
	void calculate(long long count,long long start,int chunkcount=8)
	{
		this->count=count;
		this->start=start;
		this->chunkcount=chunkcount;
		this->sum=0;
		th=thread(call,this);
	}
	long double join()
	{
		th.join(); 
		return sum;
	}
	public:
	static long double calculate(long long count,int chunkcount=8)
	{
		vector<ChunkSummator> ths(chunkcount);
		long double sum=0;
		for(int c=0;c<chunkcount;++c) ths[c].calculate(count,c,chunkcount);
		for(int c=0;c<chunkcount;++c) sum+=ths[c].join();
		return sum;
	}
};

int main()
{
	cout<<ChunkSummator::calculate(1)<<endl;
	cout<<ChunkSummator::calculate(2)<<endl;
	cout<<ChunkSummator::calculate(3)<<endl;
	cout<<ChunkSummator::calculate(4)<<endl;
	cout<<ChunkSummator::calculate(5)<<endl;
	cout<<ChunkSummator::calculate(100000)<<endl;
	cout<<ChunkSummator::calculate(100000,1)<<endl;
	return 0;
}

EDIT: Wg sugestii od @MarekR22 oraz instalacji clang'a:

#include <numeric>
#include <vector>
#include <algorithm>
#include <future>
#include <iostream>
using namespace std;

long double func(long long i) { return 1/(i*(long double)i); }

long double sumfunc(long long count,int start,int chunkcount)
{
	long double sum=0;
	for(long long i=start+1;i<=count;i+=chunkcount) sum+=func(i);
	return sum;
}

long double parallelsumfunc(long long count,int chunkcount)
{
    vector<future<long double>> parts(chunkcount);
    for(int i=0;i<chunkcount;++i) parts[i]=async(&sumfunc,count,i,chunkcount);
    return accumulate(begin(parts),end(parts),(long double)0,[](auto acc,auto &val) { return acc+val.get(); });
}

int main()
{
   cout<<" Regular sum: "<<sumfunc(5*1000*1000*1000,0,1)<<endl;
   cout<<"Pararell sum: "<<parallelsumfunc(5*1000*1000*1000,8)<<endl;
   return 0;
}

Miło, krótko, zgrabnie i przyjemnie!