Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
fuerlinger opened this issue Mar 26, 2017 · 5 comments
Assignees
Milestone

Comments

@fuerlinger
Copy link
Contributor

  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.

@rkowalewski
Copy link

rkowalewski commented Mar 27, 2017

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.

@fuerlinger
Copy link
Contributor Author

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 ?

@devreal devreal added this to the dash-0.3.0 milestone Apr 19, 2017
@fuchsto
Copy link
Member

fuchsto commented May 20, 2017

@fuerlinger Solving this using view expressions now.

The changes proposed in #347 have been rejected as commented here:
#347 (comment)

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

@fuchsto
Copy link
Member

fuchsto commented May 20, 2017

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

@devreal
Copy link
Member

devreal commented Jan 16, 2018

@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 devreal modified the milestones: dash-0.3.0, dash-0.4.0 Sep 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants