dash-project / dash

DASH, the C++ Template Library for Distributed Data Structures with Support for Hierarchical Locality for HPC and Data-Driven Science
http://www.dash-project.org/
Other
153 stars 44 forks source link

Why doesn't dash::copy support GlobPtr as input? #246

Open BenjaProg opened 7 years ago

BenjaProg commented 7 years ago

Hey, is there a reason why dash::copy doesn't accept GlobPtr as in/output? I stumbled about this as i wanted to copy to a globmem allocated with memalloc on a remote unit. it's just in the comments right now and I try to solve the problem another way, but I am still curious why ;-)

fuchsto commented 7 years ago

dash::copy is an algorithm interface, that is: it has several implementations depending on the direction (local-global, global-local, global-global) with adapter wrappers for different parameter types. GlobPtr ranges should be supported and, at least for some variants, already are. Apparently an adapter specialization for GlobPtr is missing for your use case. I will add it.

fmoessbauer commented 7 years ago

As discussed in #325 a GlobPtr does not respect the structure of values in memory. Passing a GlobPtr to dash::copy should not be allowed. Taken again the std::list example, this demonstrates why: Passing a pointer to a list element to an STL algorithm does not work because the data is not contiguous in memory. There you need full blown iterators.

Keeping that in mind, I guess we can close this issue.

fuchsto commented 7 years ago

@fmoessbauer That's right in principle, but std::copy accepts pointers and so should we.

devreal commented 7 years ago

We could annotate GlobPtr type to signal whether it points to contiguous memory (i.e., it points to a dash::Matrix or dash::Array) and at runtime error out if the user tries to copy more than one element and the memory region pointed to is not contiguous. This might be more restrictive than necessary but prevents from running total havoc in the transport layer.

fuchsto commented 7 years ago

I think it should not. GlobPtr is a dull representation of an address. Or thinking the other way around: In which situation would application programmers use dash::copy or std::copy (which is explicitly supported!) on a GlobPtr? To avoid overhead from validation and index calculations. Also, GlobPtr has no concept of containers (other than GlobIter types) so its implementation has no means to reflect container memory, like contiguous data. Hey, perhaps copying across the iteration space it's exactly what the application developer wants to do? It's not the concern of DASH algorithms or GlobPtr.

fuchsto commented 7 years ago

Update: GlobPtr<T> now provides robust global random access pointer semantics for any memory space so dash::copy can be safely extended to support global pointers now.

BenjaProg commented 7 years ago

cool, nice improvement! :-)

devreal commented 6 years ago

Is anyone working on this? @tuxamito ran into this problem today. @rkowalewski is that part of your upcoming PR?

tuxamito commented 6 years ago

I totally need this feature to be able to access arrays pointed by GlobPtr. Accessing the elements one by one is terribly slow :(