Solve segmentation fault of dash::transform on unkown types

[ddiefenthaler]

As stated by #203 dash::transform fails at runtime when operating on a type not defined in Types.h.

A suggestion was to move this error to compile time. This could be done by removing the member value from the struct dart_datatype<T> without specialization.
Unfortunaly this does not work this easy as this also fails in other situations. For example each use of GlobRef<T> depends on dart_datatype<T>.
Since DART_TYPE_UNDEFINED works fine in these different contexts, it does not seem to be the actual problem.

A possible solution could be to duplicate the member value for all specializations, but not for the base case. This could be called value_strong for example.
This duplicate could then get used in all the places where it is important that DART knows about the specific type.

[devreal]

Wouldn't it be sufficient to introduce a static_assert in dash::internal::accumulate_impl* that checks that the type is known? Something like this:

  static_assert(dash::dart_datatype<ValueType>::value != DART_TYPE_UNDEFINED,
      "Cannot accumulate unknown type!");

[fmoessbauer]

I agree that this is probably the simples solution. This statement should also be added in dash::accumulate.

[devreal]

Good point! I also wonder why accumulate_impl resides in Transform.h. Wouldn't it be more efficient for dash::accumulate to call accumulate_impl for operations we can map to DART operations instead of doing the accumulate on-by-one on global memory?

[fmoessbauer]

I stumbled upon this too, but therefor we need execution policies ( #104 ). However you are right. MPI_accumulate is much more efficient and has a better scaling.

Update: Ok, wait: According to #104 dash::transform already uses accumulate...

[devreal]

I think we can implement it using dart_accumulate without execution policies, this will be the default then.

[devreal]

Please see PR #213, which introduces the checks for valid data types.

Regarding dash::accumulate, the current implementation has a flaw: only unit 0 (randomly chosen?) actually receives a valid result. This is neither documented nor does the user have a chance to influence this. I seem to remember that we had a similar discussion at our last F2F. IMO, either the user can choose which unit receives the result or it should be broadcast to all units. At the very least, it has to be documented...

[devreal]

@ddiefenthaler Please let us know if PR #213 has fixed your problem.

[ddiefenthaler]

Fixed, thank you.
As mentioned in the first comment it was actually just a fork of Issue #203, but with a more general solution in mind.

When writing this yesterday I already thought that there must be simpler solutions, but I weren't able to think about any. However I haven't done much with static_assert prior to DASH.