1. ## MPI c problem

I've been working on an algorithm to do exercise 3 from NESL Exercises. I have been racking my brain for the past day and cannot figure why it is giving me an abort().
It is supposed to read a file with the numbers in it called test.txt and in the file
Code:
```16
12
11
10
8
5
8
9
6
7
7
10
7
4
2
4
2```
Any help would be much appreciated
I chose to do a divide and conquer method and using 16 nodes, node 15 never properly terminates for some reason:

Code:
```
max=data[mypid];
min=data[mypid];
max_id=mypid;
min_id=mypid;
diff=0;

tempmax=data[mypid];
tempmin=data[mypid];
tempmax_id=mypid;
tempmin_id=mypid;
tempdiff=0;

for( i=0; i < (log(nprocs)/log(2)); i++ )
{
if( mypid % (int)pow(2, i) == 0 )
{
if( mypid % (int)pow(2, i + 1) == 0 ) //join the work
{
MPI_Recv(&tempmax, 1, MPI_INT, (mypid+(int)pow(2,i)), 0, MPI_COMM_WORLD, &status);
MPI_Recv(&tempmax_id, 1, MPI_INT, (mypid+(int)pow(2,i)), 1, MPI_COMM_WORLD, &status);
MPI_Recv(&tempmin, 1, MPI_INT, (mypid+(int)pow(2,i)), 2, MPI_COMM_WORLD, &status);
MPI_Recv(&tempmin_id, 1, MPI_INT, (mypid+(int)pow(2,i)), 3, MPI_COMM_WORLD, &status);
MPI_Recv(&tempdiff, 1, MPI_INT, (mypid+(int)pow(2,i)), 4, MPI_COMM_WORLD, &status);

if((diff < 0)&&(tempdiff< 0))
{
max=0;
max_id=0;
if (min>tempmin)
{
min=tempmin;
min_id=tempmin_id;
}
diff=0;
}
else
{
if(diff <=0)
{
if (max<tempmax)
{
max=tempmax;
max_id=tempmax_id;
}
if (min>tempmin)
{
min=tempmin;
min_id=tempmin_id;
}
if (max_id<min_id)
{
diff=(max-min)*-1;
}
else
{
diff=max-min;
}
}
else
{
if (max<tempmax)
{
max=tempmax;
max_id=tempmax_id;
}
if (max_id<min_id)
{
diff=(max-min)*-1;
}
else
{
diff=max-min;
}
}
}
printf("P%d max=%d max_id=%d min=%d min_id=%d diff=%d \n", mypid,max,max_id,min,min_id,diff);
}

else if( mypid % (int)pow(2, i + 1) != 0 )
{
MPI_Send(&tempmax, 1, MPI_INT, (mypid+(int)pow(2,i+1)), 0, MPI_COMM_WORLD);
MPI_Send(&tempmax_id, 1, MPI_INT, (mypid+(int)pow(2,i+1)), 1, MPI_COMM_WORLD);
MPI_Send(&tempmin_id, 1, MPI_INT, (mypid+(int)pow(2,i+1)), 3, MPI_COMM_WORLD);
MPI_Send(&tempmin, 1, MPI_INT, (mypid+(int)pow(2,i+1)), 2, MPI_COMM_WORLD);
MPI_Send(&tempdiff, 1, MPI_INT, (mypid+(int)pow(2,i+1)), 4, MPI_COMM_WORLD);
}
}
}
if( input != NULL )
{
fclose(input);
}
//closes the file
free(data);
MPI_Finalize();
return 0;```

2. Since you decided to trim away any possibility that we might have to run your code, I can only suggest some ideas for debugging your own code.

Code:
```for( i=0; i < (log(nprocs)/log(2)); i++ )
{
printf("Debug: outer loop i=%d\n", i );```
Note that (log(nprocs)/log(2)) is all done in floating point, so be suspicious of rounding errors. Likewise for all your other usage of pow() as well.

I would suggest you replace raising 2 to the power of some other positive integer
pow(2, i)
would be
(1<<i)

3. sorry for cutting the code
Code:
```#include <stdio.h>
#include <mpi.h>
#include "math.h"

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

int mypid, nprocs;
int N, flag, index;
//N is used for the total number of elements in the dynamically allocated array
int temp = 0;
int sum = 0;
int modulus, numSum;
int max, max_id, min, min_id, diff;
int tempmax, tempmax_id, tempmin, tempmin_id, tempdiff;
int i;

FILE *input = NULL; //setup for using files
int *data = NULL; //used to create the dynamically allocated array

MPI_Init(&argc, &argv);
MPI_Comm_size(MPI_COMM_WORLD, &nprocs);
MPI_Comm_rank(MPI_COMM_WORLD, &mypid);
MPI_Status status;

if( mypid == 0 )
{
input = fopen("test.txt", "r");
//opens the file input.txt if it is in the root directory

if( input == NULL )
//if there is no such file
{
printf("Cannot open file [%s] \n", "input");
flag = 1;
}

if( fscanf(input, "%d", &N) != 1 )
//if the first thing is not a number
{
printf("Wrong format \n");
flag = 1;
}
}
MPI_Bcast(&flag, 1, MPI_INT, 0, MPI_COMM_WORLD);
if(flag == 1)
//something failed, all processors terminate
{
MPI_Finalize();
return 0;
}
MPI_Bcast(&N, 1, MPI_INT, 0, MPI_COMM_WORLD);
data = calloc(N, sizeof(int));
//defines the dynamic arry to size N
if( data == NULL )
//the length of data is 0
{
printf("mem allocation error \n");
MPI_Finalize();
free(data);
flag = 1;
return 0;
}
if(mypid == 0)
{
for(i = 0; i < N; ++i)
{
if(fscanf(input, "%d", data + i) != 1) //scan in numbers
{
printf("Not enough numbers as specified. \n");
MPI_Finalize();
free(data);
flag = 1;
return 0;
}
}
}
//this reads in the numbers and gives an error if there are not enough as specified

MPI_Bcast(&flag, 1, MPI_INT, 0, MPI_COMM_WORLD);

if( flag == 1)
//terminates the other processors
{
MPI_Finalize();
free(data);
return 0;
}

MPI_Bcast(data, N, MPI_INT, 0, MPI_COMM_WORLD);

max=data[mypid];
min=data[mypid];
max_id=mypid;
min_id=mypid;
diff=0;

tempmax=data[mypid];
tempmin=data[mypid];
tempmax_id=mypid;
tempmin_id=mypid;
tempdiff=0;

for( i=0; i < (log(nprocs)/log(2)); i++ )
{
if( mypid % (int)pow(2, i) == 0 )
{
if( mypid % (int)pow(2, i + 1) == 0 ) //join the work
{
MPI_Recv(&tempmax, 1, MPI_INT, (mypid+(int)pow(2,i)), 0, MPI_COMM_WORLD, &status);
MPI_Recv(&tempmax_id, 1, MPI_INT, (mypid+(int)pow(2,i)), 1, MPI_COMM_WORLD, &status);
MPI_Recv(&tempmin, 1, MPI_INT, (mypid+(int)pow(2,i)), 2, MPI_COMM_WORLD, &status);
MPI_Recv(&tempmin_id, 1, MPI_INT, (mypid+(int)pow(2,i)), 3, MPI_COMM_WORLD, &status);
MPI_Recv(&tempdiff, 1, MPI_INT, (mypid+(int)pow(2,i)), 4, MPI_COMM_WORLD, &status);

if((diff < 0)&&(tempdiff< 0))
{
max=0;
max_id=0;
if (min>tempmin)
{
min=tempmin;
min_id=tempmin_id;
}
diff=0;
}
else
{
if(diff <=0)
{
if (max<tempmax)
{
max=tempmax;
max_id=tempmax_id;
}
if (min>tempmin)
{
min=tempmin;
min_id=tempmin_id;
}
if (max_id<min_id)
{
diff=(max-min)*-1;
}
else
{
diff=max-min;
}
}
else
{
if (max<tempmax)
{
max=tempmax;
max_id=tempmax_id;
}
if (max_id<min_id)
{
diff=(max-min)*-1;
}
else
{
diff=max-min;
}
}
}
printf("P%d max=%d max_id=%d min=%d min_id=%d diff=%d \n", mypid,max,max_id,min,min_id,diff);
}

else if( mypid % (int)pow(2, i + 1) != 0 )
{
MPI_Send(&tempmax, 1, MPI_INT, (mypid+(int)pow(2,i+1)), 0, MPI_COMM_WORLD);
MPI_Send(&tempmax_id, 1, MPI_INT, (mypid+(int)pow(2,i+1)), 1, MPI_COMM_WORLD);
MPI_Send(&tempmin_id, 1, MPI_INT, (mypid+(int)pow(2,i+1)), 3, MPI_COMM_WORLD);
MPI_Send(&tempmin, 1, MPI_INT, (mypid+(int)pow(2,i+1)), 2, MPI_COMM_WORLD);
MPI_Send(&tempdiff, 1, MPI_INT, (mypid+(int)pow(2,i+1)), 4, MPI_COMM_WORLD);
}
}
}
if( input != NULL )
{
fclose(input);
}
//closes the file
free(data);
MPI_Finalize();
return 0;
}```

4. So did you do any testing like I suggested?

5. Yes, I just tried it and it is always a whole number because I am using 16 nodes for the time being. I tried replacing it with i<4, i tried making i a double type. I had to take out all the code in the outer loop just to get it to run:

Code:
```for( i=0; i < (log(nprocs)/log(2)); i++ )
{
printf("debugging outer loop i=%d\n",i)
}
if( input != NULL )
{
fclose(input);
}
//closes the file
free(data);
MPI_Finalize();
return 0;
}
```

6. For i == 0 and mypid == 15 destination in MPI_Send is out of the ranks' range:
Code:
```    for( i=0; i < (log(nprocs)/log(2)); i++ )
{
if( mypid % (int)pow(2, i) == 0 )
{
if( mypid % (int)pow(2, i + 1) == 0 ) //join the work
{
..........
}

else if( mypid % (int)pow(2, i + 1) != 0 )
{
MPI_Send(&tempmax, 1, MPI_INT, (mypid+(int)pow(2,i+1)), 0, MPI_COMM_WORLD);
MPI_Send(&tempmax_id, 1, MPI_INT, (mypid+(int)pow(2,i+1)), 1, MPI_COMM_WORLD);
MPI_Send(&tempmin_id, 1, MPI_INT, (mypid+(int)pow(2,i+1)), 3, MPI_COMM_WORLD);
MPI_Send(&tempmin, 1, MPI_INT, (mypid+(int)pow(2,i+1)), 2, MPI_COMM_WORLD);
MPI_Send(&tempdiff, 1, MPI_INT, (mypid+(int)pow(2,i+1)), 4, MPI_COMM_WORLD);
}
}
}```
Other problem:
Code:
```    if(mypid == 0)
{
for(i = 0; i < N; ++i)
{
if(fscanf(input, "%d", data + i) != 1) //scan in numbers
{
printf("Not enough numbers as specified. \n");
MPI_Finalize();
free(data);
flag = 1;
return 0;
}
}
}
//this reads in the numbers and gives an error if there are not enough as specified

MPI_Bcast(&flag, 1, MPI_INT, 0, MPI_COMM_WORLD);```
Node with mypid == 0 returns if fscanf error happens, then you call broadcasting of the error flag from this node.

7. This might sound stupid but how would go about fixing the first problem? I already fixed the second one. Thank you.

8. Nevermind, i got it. Thank you for all your help, though i now have a logic problem ^^;;;

9. Here's the completed project (I don't care about sharing my code with people) can anyone think of any way to improve this code? or simplify it?
Code:
```#include <stdio.h>
#include <mpi.h>
#include "math.h"

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

int mypid, nprocs;
int N, flag, index;
//N is used for the total number of elements in the dynamically allocated array
int temp = 0;
int sum = 0;
int modulus, numSum;
int max, max_id, min, min_id, diff;
int tempmax, tempmax_id, tempmin, tempmin_id, tempdiff;
int i;

FILE *input = NULL; //setup for using files
int *data = NULL; //used to create the dynamically allocated array

MPI_Init(&argc, &argv);
MPI_Comm_size(MPI_COMM_WORLD, &nprocs);
MPI_Comm_rank(MPI_COMM_WORLD, &mypid);
MPI_Status status;

if( mypid == 0 )
{
input = fopen("test.txt", "r");
//opens the file input.txt if it is in the root directory

if( input == NULL )
//if there is no such file
{
printf("Cannot open file [%s] \n", "input");
flag = 1;
}

if( fscanf(input, "%d", &N) != 1 )
//if the first thing is not a number
{
printf("Wrong format \n");
flag = 1;
}
}
MPI_Bcast(&flag, 1, MPI_INT, 0, MPI_COMM_WORLD);
if(flag == 1)
//something failed, all processors terminate
{
MPI_Finalize();
return 0;
}
MPI_Bcast(&N, 1, MPI_INT, 0, MPI_COMM_WORLD);
data = calloc(N, sizeof(int));
//defines the dynamic arry to size N
if( data == NULL )
//the length of data is 0
{
printf("mem allocation error \n");
MPI_Finalize();
free(data);
flag = 1;
return 0;
}
if(mypid == 0)
{
for(i = 0; i < N; ++i)
{
if(fscanf(input, "%d", data + i) != 1) //scan in numbers
{
printf("Not enough numbers as specified. \n");
free(data);
flag = 1;
return 0;
}
}
}
//this reads in the numbers and gives an error if there are not enough as specified

MPI_Bcast(&flag, 1, MPI_INT, 0, MPI_COMM_WORLD);

if( flag == 1)
//terminates the other processors
{
MPI_Finalize();
free(data);
return 0;
}

MPI_Bcast(data, N, MPI_INT, 0, MPI_COMM_WORLD);

max=data[mypid];
min=data[mypid];
max_id=mypid;
min_id=mypid;
diff=0;

tempmax=data[mypid];
tempmin=data[mypid];
tempmax_id=mypid;
tempmin_id=mypid;
tempdiff=0;

for( i=0; i<(log(nprocs)/log(2)); i++ )
{
if( mypid % (int)pow(2, i) == 0 )
{
if( mypid % (int)pow(2, i + 1) == 0 ) //join the work
{

MPI_Recv(&tempmax, 1, MPI_INT, (mypid+(int)pow(2,i)), 0, MPI_COMM_WORLD, &status);
MPI_Recv(&tempmax_id, 1, MPI_INT, (mypid+(int)pow(2,i)), 1, MPI_COMM_WORLD, &status);
MPI_Recv(&tempmin, 1, MPI_INT, (mypid+(int)pow(2,i)), 2, MPI_COMM_WORLD, &status);
MPI_Recv(&tempmin_id, 1, MPI_INT, (mypid+(int)pow(2,i)), 3, MPI_COMM_WORLD, &status);
MPI_Recv(&tempdiff, 1, MPI_INT, (mypid+(int)pow(2,i)), 4, MPI_COMM_WORLD, &status);

if((diff < 0)&&(tempdiff < 0))
{
max=0;
max_id=0;
if (min>tempmin)
{
min=tempmin;
min_id=tempmin_id;
}
diff=0;
tempmax=max;
tempmax_id=max_id;
tempmin=min;
tempmin_id=min_id;
tempdiff=diff;
}
else
{
if((diff <= 0))
{
if (max<tempmax)
{
max=tempmax;
max_id=tempmax_id;
}
else if (max==tempmax)
{
max=tempmax;
max_id=tempmax_id;
}
if (min>tempmin)
{
min=tempmin;
min_id=tempmin_id;
}
if (max_id<min_id)
{
diff=(max-min)*-1;
}
else
{
diff=max-min;
}
}
else
{
if (max<tempmax)
{
max=tempmax;
max_id=tempmax_id;
}
if (max_id<min_id)
{
diff=(max-min)*-1;
}
else
{
diff=max-min;
}
}
tempmax=max;
tempmax_id=max_id;
tempmin=min;
tempmin_id=min_id;
tempdiff=diff;
}
//				printf("P%d max=%d max_id=%d min=%d min_id=%d diff=%d on i=%d\n", mypid,max,max_id,min,min_id,diff,i);

}

else if( mypid % (int)pow(2, i + 1) != 0 )
{

MPI_Send(&tempmax, 1, MPI_INT, (mypid-(int)pow(2,i)), 0, MPI_COMM_WORLD);
MPI_Send(&tempmax_id, 1, MPI_INT, (mypid-(int)pow(2,i)), 1, MPI_COMM_WORLD);
MPI_Send(&tempmin_id, 1, MPI_INT, (mypid-(int)pow(2,i)), 3, MPI_COMM_WORLD);
MPI_Send(&tempmin, 1, MPI_INT, (mypid-(int)pow(2,i)), 2, MPI_COMM_WORLD);
MPI_Send(&tempdiff, 1, MPI_INT, (mypid-(int)pow(2,i)), 4, MPI_COMM_WORLD);

}
}

}
if (mypid==0)
{
printf("The best day to buy the stock is day %d for %d and sell on day %d for %d making %d\n", min_id+1, min, max_id+1, max, diff);
}

if( input != NULL )
{
fclose(input);
}
//closes the file
free(data);
MPI_Finalize();
return 0;
}```

10. Code:
```if( input == NULL )
//if there is no such file
{
printf("Cannot open file [%s] \n", "input");
flag = 1;
}

if( fscanf(input, "%d", &N) != 1 )
//if the first thing is not a number
{
printf("Wrong format \n");
flag = 1;
}

// Line 4:  Don't you want the filename to be printed (i.e. "test.txt") instead of "input"?
// Line 8:  You check if the file opened successfully on line 1, but regardless of whether
//          or not it did, the "fscanf()" will be called anyway.```
I didn't really look at anything past that, though.

11. For me, splitting the code into two or three functions would help with the overall program clarity.
200+ lines in main is a bit much.

12. If I were to do split it, it would be a file checking/reading function, and one for the actual finding.
I could also remove some of the unused variables too like modulus and numSum

13. That sounds like a good start.

14. You seem to have skipped this earlier suggestion of replacing this:
Code:
`(int)pow(2,i)`
with this:
Code:
`(1 << i)`
In terms of efficiency, the first one is like cycling to the train station, taking the train to the next stop, then cycling all the way back to your neighbours house (which is your intended destination).
The later is like just walking next door.

15. did not know what the '<<' was for and doing a simple google search i could not find it.