[mpich-discuss] initialization issue in strided_get_indexed

[Jim Dinan]

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