[mpich-discuss] initialization issue in strided_get_indexed

[Jim Dinan]

Hi Sreeram,

I committed a fix to the MPICH2 trunk (r9335).  I added the missing 
initializations and set local buffers to -1.0 to make the tests more 
robust when run on a single process.  I also fixed those bad floating 
point comparisons.

Thanks again for reporting this.

Best wishes,
  ~Jim.

On 12/12/11 4:16 PM, Jim Dinan wrote:
> Hi Sreeram,
>
> Your understanding of the test case is spot on; I just didn't read it
> closely enough before responding. I think this bug is the culprit:
>
> if (actual - expected > 1e-10) {
>
> This should have an fabs(), somehow we've just been getting lucky:
>
> if (fabs(actual - expected) > 1e-10) {
>
> This affects all of those strided tests. I'll put together a fix and let
> you know as soon as it's checked in. Great catch and thanks for the bug
> report!
>
> Best,
> ~Jim.
>
>
> On 12/12/11 3:47 PM, sreeram potluri wrote:
>> Jim,
>>
>> My understanding of the test is that only a part of the loc_buf is
>> overwritten by the MPI_Get operation (8*256 rather than 8*1024).
>>
>> Now the test also tries to validate the rest of the loc_buf which is not
>> affected by the MPI_Get operation. This check can fail or pass based on
>> the original value of the loc_buf, which is never set.
>>
>> Am I missing something basic?
>>
>> Sreeram Potluri
>>
>> On Mon, Dec 12, 2011 at 3:11 PM, Jim Dinan <dinan at mcs.anl.gov
>> <mailto:dinan at mcs.anl.gov>> wrote:
>>
>> Hi Sreeram,
>>
>> Setting the initial value of loc_buf shouldn't influence whether or
>> not this test passes; the entire contents of loc_buf should be
>> overwritten later by the call to MPI_Get. As Dave pointed out,
>> setting this to 1.0+rank also weakens the test when run on one
>> process since loc_buf will already contain the correct data. This
>> is not the case when run on more processes since loc_buf should
>> contain 1.0+((rank+1)%nproc)).
>>
>> This test has been in the MPICH test suite for several months and
>> hasn't given us any trouble. Any chance that it's exercising a bug
>> on your end?
>>
>> Best,
>> ~Jim.
>>
>>
>> On 12/12/11 1:27 PM, Dave Goodell wrote:
>>
>> Sreeram,
>>
>> I'm not very familiar with this test, but this looks like an
>> incorrect change to me. By setting the loc_buf (the origin
>> "receive" buffer) to the expected value in this way then you
>> open up a testing coverage gap if MPI_Get is replaced with a
>> no-op function.
>>
>> If anything, this test could probably be strengthened by adding
>> the following line in the same location that you recommend
>> making your change:
>>
>> ----8<----
>> *(loc_buf + i) = -1.0;
>> ----8<----
>>
>> This way you don't assume anything about the values already in
>> the buffer returned by MPI_Alloc_mem.
>>
>> Perhaps Jim can comment, since he wrote the test originally.
>>
>> -Dave
>>
>> On Dec 12, 2011, at 6:53 AM CST, sreeram potluri wrote:
>>
>> Hi Team,
>>
>> We have seen a validation error with the strided_get_indexed
>> test in our internal testing with MVAPICH2. This appears to
>> be an initialization issue with the benchmark. The path
>> below fixes it in one way. Please accept the change if
>> appropriate.
>>
>> Index: mpi/rma/strided_get_indexed.c
>> ==============================__==============================__=======
>> --- mpi/rma/strided_get_indexed.c (revision 5064)
>> +++ mpi/rma/strided_get_indexed.c (working copy)
>> @@ -48,8 +48,10 @@
>> if (rank == 0)
>> if (verbose) printf("MPI RMA Strided Get Test:\n");
>>
>> - for (i = 0; i< XDIM*YDIM; i++)
>> + for (i = 0; i< XDIM*YDIM; i++) {
>> *(win_buf + i) = 1.0 + rank;
>> + *(loc_buf + i) = 1.0 + rank;
>> + }
>>
>> MPI_Win_create(win_buf, bufsize, 1, MPI_INFO_NULL,
>> MPI_COMM_WORLD,&buf_win);
>>
>> Regards
>> Sreeram Potluri
>> _________________________________________________
>> mpich-discuss mailing list mpich-discuss at mcs.anl.gov
>> <mailto:mpich-discuss at mcs.anl.gov>
>> To manage subscription options or unsubscribe:
>> https://lists.mcs.anl.gov/__mailman/listinfo/mpich-discuss
>> <https://lists.mcs.anl.gov/mailman/listinfo/mpich-discuss>
>>
>>
>> _________________________________________________
>> mpich-discuss mailing list mpich-discuss at mcs.anl.gov
>> <mailto:mpich-discuss at mcs.anl.gov>
>> To manage subscription options or unsubscribe:
>> https://lists.mcs.anl.gov/__mailman/listinfo/mpich-discuss
>> <https://lists.mcs.anl.gov/mailman/listinfo/mpich-discuss>
>>
>>
>>
>>
>> _______________________________________________
>> mpich-discuss mailing list mpich-discuss at mcs.anl.gov
>> To manage subscription options or unsubscribe:
>> https://lists.mcs.anl.gov/mailman/listinfo/mpich-discuss
> _______________________________________________
> mpich-discuss mailing list mpich-discuss at mcs.anl.gov
> To manage subscription options or unsubscribe:
> https://lists.mcs.anl.gov/mailman/listinfo/mpich-discuss