-
Notifications
You must be signed in to change notification settings - Fork 48
Honor output range distribution in dash::transform #398
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
base: development
Are you sure you want to change the base?
Conversation
bertwesarg
left a comment
There was a problem hiding this 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(); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused?
|
Perfect, thanks a lot! |
dash::copy, needed fordash::transformthat honors the distribution of the output range for local input ranges, whichstd::vectorFixes #386