dash icon indicating copy to clipboard operation
dash copied to clipboard

dash::copy (local range to global destination) not copying the whole range

Open fuerlinger opened this issue 7 years ago • 5 comments

  dash::Array<int> arr(100);

  if( dash::myid()==0 ) {
    int buf[100];
    std::iota(buf, buf+100, 0);
    // copy local buffer to global array
    dash::copy(buf, buf+100, arr.begin());
  }
  arr.barrier();

  if( dash::myid()==0 ) {
    for( auto el: arr ) {
      cout << (int)el << " ";
    }
    cout << endl;
  }

Looks like the copy is performed only for the fist two units and the other transfers go haywire somewhere - CMake builds give an additional segfault, a build with the manual Makefile, strangely, doesn't.

mpirun -n 4 ./main
0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
mpirun -n 5 ./main
0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0

std::copy works as expected (good!) but of course then the transfer is done element-by-element.

fuerlinger avatar Mar 26 '17 17:03 fuerlinger

See pull request #347 for a proposed solution. As mentioned there, it does not consider fragmented local memory layout but assumes contiguous local ranges. I am not sure if we support this in the other dash::copy algorithms.

rkowalewski avatar Mar 27 '17 12:03 rkowalewski

Bump! dash::copy for local to global is still not working. Issues of async aside, why can't we use @rkowalewski 's solution from #347 ?

fuerlinger avatar Apr 12 '17 09:04 fuerlinger

@fuerlinger Solving this using view expressions now.

The changes proposed in #347 have been rejected as commented here: https://github.com/dash-project/dash/pull/347#issuecomment-289970209

In brief: It doesn't work for non-contiguous elements (e.g. cyclic distribution) and it's inefficient.

fuchsto avatar May 20 '17 01:05 fuchsto

Addressed in #410

New implementation uses view expressions. If find this diff quite convincing: https://github.com/dash-project/dash/pull/410/files#diff-c4c2bc3b549c09735c61251a8f8d8ecfR1149

fuchsto avatar May 20 '17 11:05 fuchsto

@fuchsto When can we expect #410 to land? Local-to-global clearly is broken (or incomplete, at least) in development and I'd consider this a road-block for 0.3.0. So either we fix it in development or wait for view expressions to get stable...

devreal avatar Jan 16 '18 04:01 devreal