Discussion:
MPI_Bcast problem
(too old to reply)
h***@gmail.com
2008-05-23 13:36:30 UTC
Permalink
Hello,

I'm trying to broadcast a 4MB of data, but even though I followed all
the guidelines for using MPI_Bcast the slaves buffers don't get the
values from root.
I have a master (rank 0), result manager (rank 1) and some slaves.
After some number crunching they all get to following lines

MPI_Barrier(MPI_COMM_WORLD);
MPI_Bcast(Bhalf->data[0], Bhalf->rows*Bhalf->cols, MPI_FLOAT, 0,
MPI_WORLD_COMM);
printf("bcasted, rank:%i [0][0]=%f size:%u\n",rank, Bhalf->data[0][0],
Bhalf->rows*Bhalf->cols);

but the output reads

bcasted, rank:0 [0][0]=27.000000 size:1048576
bcasted, rank:1 [0][0]=0.000000 size:1048576
bcasted, rank:2 [0][0]=0.000000 size:1048576
bcasted, rank:4 [0][0]=0.000000 size:1048576
bcasted, rank:3 [0][0]=0.000000 size:1048576
bcasted, rank:10 [0][0]=0.000000 size:1048576
bcasted, rank:8 [0][0]=0.000000 size:1048576
bcasted, rank:7 [0][0]=0.000000 size:1048576
bcasted, rank:9 [0][0]=0.000000 size:1048576
bcasted, rank:6 [0][0]=0.000000 size:1048576
bcasted, rank:5 [0][0]=0.000000 size:1048576

I have verified via a debugger that they in fact meet at the bcast at
the same time, that the Bhalf is initialized with the same values.
Dave Seaman
2008-05-23 19:09:19 UTC
Permalink
Post by h***@gmail.com
Hello,
I'm trying to broadcast a 4MB of data, but even though I followed all
the guidelines for using MPI_Bcast the slaves buffers don't get the
values from root.
I have a master (rank 0), result manager (rank 1) and some slaves.
After some number crunching they all get to following lines
MPI_Barrier(MPI_COMM_WORLD);
MPI_Bcast(Bhalf->data[0], Bhalf->rows*Bhalf->cols, MPI_FLOAT, 0,
MPI_WORLD_COMM);
printf("bcasted, rank:%i [0][0]=%f size:%u\n",rank, Bhalf->data[0][0],
Bhalf->rows*Bhalf->cols);
The first argument to MPI_Bcast is supposed to be a pointer to the data
buffer. In the absence of declarations it is difficulat to say for
certain, but your first argument looks like a scalar (an array element)
and not a pointer.
--
Dave Seaman
Third Circuit ignores precedent in Mumia Abu-Jamal ruling.
<http://www.indybay.org/newsitems/2008/03/29/18489281.php>
d***@csclub.uwaterloo.ca.invalid
2008-05-23 19:27:39 UTC
Permalink
Post by Dave Seaman
Post by h***@gmail.com
Hello,
I'm trying to broadcast a 4MB of data, but even though I followed all
the guidelines for using MPI_Bcast the slaves buffers don't get the
values from root.
I have a master (rank 0), result manager (rank 1) and some slaves.
After some number crunching they all get to following lines
MPI_Barrier(MPI_COMM_WORLD);
MPI_Bcast(Bhalf->data[0], Bhalf->rows*Bhalf->cols, MPI_FLOAT, 0,
MPI_WORLD_COMM);
printf("bcasted, rank:%i [0][0]=%f size:%u\n",rank, Bhalf->data[0][0],
Bhalf->rows*Bhalf->cols);
The first argument to MPI_Bcast is supposed to be a pointer to the data
buffer. In the absence of declarations it is difficulat to say for
certain, but your first argument looks like a scalar (an array element)
and not a pointer.
That's unlikely in this case.
A few lines down Bhalf->data is indexed as a two-dimensional array, so
Bhalf->data[0] is (should be) an array, which will decay to a pointer
to its first element (which is what the OP wants).

One potentially-suspicious thing would be if Bhalf->data is a pointer
to a vector of pointers (or, equivalently for our purposes, an array of
pointers) and not an actual array, but if all of those pointers point
into the same contiguous data vector, then Bhalf->data[0] is
(correctly) a pointer to the beginning. So the data would have to be
broken up into multiple different chunks for anything to break because
of that.


But based on the misspelling of MPI_COMM_WORLD in the call to
MPI_Bcast, this probably isn't actually the code the OP is running.
Remote debugging is a lot easier when we at least get to see the exact
same code as the compiler.


dave
--
Dave Vandervies dj3vande at eskimo dot com
I agree to that these concepts are horribly confusing (*).
(*) To those who don't understand the language.
--David T. Ashley in comp.lang.c
h***@gmail.com
2008-05-26 20:50:05 UTC
Permalink
Post by d***@csclub.uwaterloo.ca.invalid
That's unlikely in this case.
A few lines down Bhalf->data is indexed as a two-dimensional array, so
Bhalf->data[0] is (should be) an array, which will decay to a pointer
to its first element (which is what the OP wants).
One potentially-suspicious thing would be if Bhalf->data is a pointer
to a vector of pointers (or, equivalently for our purposes, an array of
pointers) and not an actual array, but if all of those pointers point
into the same contiguous data vector, then Bhalf->data[0] is
(correctly) a pointer to the beginning.  So the data would have to be
broken up into multiple different chunks for anything to break because
of that.
But based on the misspelling of MPI_COMM_WORLD in the call to
MPI_Bcast, this probably isn't actually the code the OP is running.
Remote debugging is a lot easier when we at least get to see the exact
same code as the compiler.
dave
--
Dave Vandervies                 dj3vande at eskimo dot com
I agree to that these concepts are horribly confusing (*).
(*) To those who don't understand the language.
                          --David T. Ashley in comp.lang.c
Haha, I expected it will be something really trivial but not that
trivial! A typo in the comm. damn!
BTW thanks a lot to all
And yes it is an array looks like this

struct Matrix
{
unsigned rows, cols;
float** data;
float* datablock;
};

initiated like this


struct Matrix* ret= (struct Matrix*) malloc(sizeof(struct Matrix));
unsigned r;

ret->cols = ccols;
ret->rows = crows;

ret->data = (float **) malloc(ret->rows * sizeof(float *));
ret->datablock = malloc_aligned_pointer(&(ret->data[0]),ret->cols*ret-
Post by d***@csclub.uwaterloo.ca.invalid
rows,cache_align);
for (r = 1; r < ret->rows; r++) ret->data[r] = ret->data[r-1] + ret-
Post by d***@csclub.uwaterloo.ca.invalid
cols;
return ret;


So its contiguous and can be accessed by index. Not my idea :)
d***@csclub.uwaterloo.ca.invalid
2008-05-26 21:31:35 UTC
Permalink
[Original code reinserted for context:]
Post by h***@gmail.com
Post by d***@csclub.uwaterloo.ca.invalid
Post by h***@gmail.com
MPI_Barrier(MPI_COMM_WORLD);
MPI_Bcast(Bhalf->data[0], Bhalf->rows*Bhalf->cols, MPI_FLOAT, 0,
MPI_WORLD_COMM);
But based on the misspelling of MPI_COMM_WORLD in the call to
MPI_Bcast, this probably isn't actually the code the OP is running.
Remote debugging is a lot easier when we at least get to see the exact
same code as the compiler.
Haha, I expected it will be something really trivial but not that
trivial! A typo in the comm. damn!
MPI_WORLD_COMM isn't defined by MPI, and it's hard to imagine a
sensible reason for a programmer to have defined something that's so
similar to (and easily confused with) something that *is* defined by
MPI.
If your compiler didn't complain about it, though, that indicates that
it did get defined somewhere, for some reason, as something that was
close enough to what you wanted that the compiler didn't notice the
difference.

So if the code you posted actually compiled cleanly, you should find
out who defined MPI_WORLD_COMM in your code, hunt them down, and break
their fingers. (Or their legs, but fingers makes it a bit harder for
them to re-offend before they've had a chance to contemplate the value
of writing non-confusing code.)
Post by h***@gmail.com
And yes it is an array looks like this
struct Matrix
{
unsigned rows, cols;
float** data;
float* datablock;
};
[with datablock pointing to a vector containing the entire dataset, and
data a vector of row pointers into datablock]
Post by h***@gmail.com
So its contiguous and can be accessed by index. Not my idea :)
It's a good one, though. That's probably the optimal combination of
flexibility and ease-of-use for dynamically allocated multidimensional
arrays in C.



dave
--
Dave Vandervies dj3vande at eskimo dot com
Post by h***@gmail.com
You can quit emacs?
Yes, and after 12 weeks of rehab, you might even stay off the stuff.
--James Riden and Greg Andrews in the scary devil monastery
h***@gmail.com
2008-05-27 09:05:08 UTC
Permalink
Post by d***@csclub.uwaterloo.ca.invalid
So if the code you posted actually compiled cleanly, you should find
out who defined MPI_WORLD_COMM in your code, hunt them down, and break
their fingers.  (Or their legs, but fingers makes it a bit harder for
them to re-offend before they've had a chance to contemplate the value
of writing non-confusing code.)
Oops my poor fingers, I'll never use find and replace again to
introduce stupid typos in my code again.
Post by d***@csclub.uwaterloo.ca.invalid
Post by h***@gmail.com
And yes it is an array looks like this
struct Matrix
{
   unsigned rows, cols;
   float** data;
   float* datablock;
};
[with datablock pointing to a vector containing the entire dataset, and
data a vector of row pointers into datablock]
It's actually even smarter. The data is aligned to a cache line by
this nasty piece of code:

float* malloc_aligned_pointer(float** p,int size, int alignment)
{
float* addr, *allocblock = (float*)calloc(size+alignment/
sizeof(float)
,sizeof(float));
addr = allocblock;
if (((long)(addr)) % alignment !=0)
addr = (float*)(((long)(addr)) + (alignment - ((long)(addr)) %
alignment ));
*p = addr;
return allocblock;

}

and the datablock points to the beginning of the allocated block
(cause I can't free the data - it could happen that it doesn't point
to a heap block)
This aligned allocation is my idea, i think i'll have to to break my
fingers again :(
d***@csclub.uwaterloo.ca.invalid
2008-05-28 16:23:57 UTC
Permalink
Post by h***@gmail.com
In article
So if the code you posted actually compiled cleanly, you should find
out who defined MPI_WORLD_COMM in your code, hunt them down, and break
their fingers. (Or their legs, but fingers makes it a bit harder for
them to re-offend before they've had a chance to contemplate the value
of writing non-confusing code.)
Oops my poor fingers, I'll never use find and replace again to
introduce stupid typos in my code again.
Heh.
I'm curious about what the find-and-replace was and what its intended
purpose was. Especially the parts that would shed light on how
MPI_WORLD_COMM got defined and what it ended up getting defined to.
(I value my fingers too, so knowing what sort of errors can get made
and how to avoid them is Always A Good Thing.)
Post by h***@gmail.com
This aligned allocation is my idea, i think i'll have to to break my
fingers again :(
Without taking a close look at it, it doesn't look all that bad. So
maybe a tap on the head with the cluestick from somebody who takes a
closer look than I did, but no broken fingers for that one.

For big arrays I'm not sure how much difference it would make, but
there are people who are smarter than I am and get paid more than I do
just to look at the effects that kind of thing have on speed for big
arrays, so if you really want to know whether it's a Good Idea and/or
done properly in your code, you'd have to find one of those people and
ask them about it.


dave
--
Dave Vandervies dj3vande at eskimo dot com
Frankly, should the entirety of Cigna's executive management die in a series
of unlikely accidents, I'd throw a party. And wonder out loud if anyone
deserved a reward for acts of public service. --Shalon Wood in the SDM
Jan Hudecek
2008-05-30 16:04:52 UTC
Permalink
Post by d***@csclub.uwaterloo.ca.invalid
Heh.
I'm curious about what the find-and-replace was and what its intended
purpose was.  Especially the parts that would shed light on how
MPI_WORLD_COMM got defined and what it ended up getting defined to.
(I value my fingers too, so knowing what sort of errors can get made
and how to avoid them is Always A Good Thing.)
well the whole thing was pretty dumb. I thought that it would be cool
to have group of processes for the bcast, cause 1 process didn't need
it. So I've #defined DATA_GROUP as MPI_COMM_WORLD +1 and i thought i
would use it as an identifier. Then I wrote DATA_GROUP to the bcast
and then I looked at the specs and realized that creating a group is
not worth the trouble.
So I wanted to remove all the references to that "group" DATA_GROUP
and now comes the critical part. Find and replace of DATA_GROUP for
hmm this world thingy MPI_WORLD_COMM. I have no idea why I didn't
remove the define, tho. I guess it must have been late at night or
what...
Post by d***@csclub.uwaterloo.ca.invalid
For big arrays I'm not sure how much difference it would make
It actually makes qiute a huge difference, because I use SSE
instructions and they need data aligned on 16-byte boundaries. So if
it weren't aligned it would crash :) And when I was doing the
alignment I thought let's go for a cache line :)

Jan
d***@csclub.uwaterloo.ca.invalid
2008-05-31 23:19:09 UTC
Permalink
Post by Jan Hudecek
well the whole thing was pretty dumb. I thought that it would be cool
to have group of processes for the bcast, cause 1 process didn't need
it. So I've #defined DATA_GROUP as MPI_COMM_WORLD +1 and i thought i
would use it as an identifier. Then I wrote DATA_GROUP to the bcast
and then I looked at the specs and realized that creating a group is
not worth the trouble.
Even if you did want to create a new group, I don't think that's the
right way to do it. Group descriptors and such things are opaque
objects that you should be using MPI calls to create.
(What you're describing looks to me like precisely the sort of thing
MPI_Comm_split is designed for.)


[Aligning memory allocations on cache lines]
Post by Jan Hudecek
Post by d***@csclub.uwaterloo.ca.invalid
For big arrays I'm not sure how much difference it would make
It actually makes qiute a huge difference, because I use SSE
instructions and they need data aligned on 16-byte boundaries. So if
it weren't aligned it would crash :) And when I was doing the
alignment I thought let's go for a cache line :)
Yep, aligning the data well enough to not crash is probably important.
(I'd expect malloc to do that for you, since it's guaranteed to return
a pointer suitably aligned for any data type, but the library may not
be aware that you're planning to use instructions with stricter-than-
normal alignment requirements.)
But once you're into the first full cache line the array uses, I would
expect the caching behavior to be the same whether or not the beginning
is aligned to a cache line boundary.


dave
--
Dave Vandervies dj3vande at eskimo dot com

Who needs alt.tasteless.pictures when you have Canadian packs of cigarettes?
--Dave Brown in the scary devil monastery
Loading...