Skip to content

Conversation

@devreal
Copy link
Member

@devreal devreal commented May 10, 2017

  1. Implement global-to-global dash::copy, needed for
  2. dash::transform that honors the distribution of the output range for local input ranges, which
  3. now also works with std iterators, .e.g, coming from std::vector

Fixes #386

Copy link
Member

@bertwesarg bertwesarg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yet tested.

auto out_it = out_first;
auto in_it = in_first;
while (towrite > 0) {
auto lpos = out_it.lpos();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't the local_size covers the total number of elements in the that unit? What if the unit has multiple local blocks?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is what I already mentioned multiple times. This approach only works for a single continuous index range. Hence it would be better to use the new range based views interface by @fuchsto . There should be something like dash::chunks(dash::local(...)). Unfortunately I do not have detailed knowledge in this API either.

Copy link
Member

@fuchsto fuchsto May 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fmoessbauer @bertwesarg
Yes, almost, it's
dash::blocks(dash::local(view_or_range_expr))
or
dash::local(dash::blocks(view_or_range_expr)).
Containers are valid range expressions, iterators can be converted to ranges using
dash::make_range(it_begin, it_end).

The term chunks was intended for any ad-hoc user-specified decomposition like dash::chunks(dash::stride(10, array)), blocks depend on the pattern. But actually there is no reason to differentiate between those.

dash::for_each_with_index(global_v.begin() + 1, global_v.end(),
[](value_t val, size_t idx){
ASSERT_EQ_U(val, (dash::size() - 1) * 1000 + (idx - 1));
++idx;
Copy link
Member

@bertwesarg bertwesarg May 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove ++idx.

dart_gptr_t dest_gptr = out_first.dart_gptr();
// Send accumulate message:
trace.enter_state("transform_blocking");
dash::internal::transform_blocking_impl(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused?

@fuchsto
Copy link
Member

fuchsto commented Jun 2, 2017

@devreal I merged your changes into PR #410 to fix this on a "substantial" level using views / ranges, your unit test are especially useful for this.

@devreal
Copy link
Member Author

devreal commented Jun 2, 2017

Perfect, thanks a lot!

@devreal devreal modified the milestones: dash-0.3.0, dash-0.4.0 Mar 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants