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

TBB implementation of reduce_by_key incompatible with transform_output_iterator #821

Open
harrism opened this issue Oct 11, 2022 · 1 comment
Labels
thrust For all items related to Thrust.

Comments

@harrism
Copy link
Contributor

harrism commented Oct 11, 2022

As part of NVIDIA/thrust#1805 I added a new test that combines reduce_by_key with transform_output_iterator. This fails to compile due to this line:

https://github.com/NVIDIA/thrust/blob/d641d63802d08c17bde9498677be300cb4277bdf/thrust/system/tbb/detail/reduce_by_key.inl#L329

I have disabled that test for the TBB backend using the Cmake macro thrust_declare_test_restrictions(). Once this issue is fixed that test should be enabled for TBB.

@gevtushenko
Copy link
Collaborator

I can reproduce this issue. The algorithm looks as follows:

  1. Input sequence is divided between P threads
  2. Each thread reduces it's chunk of data in reverse until a key changes
  3. Accumulated value is stored in carries array
  4. The rest of the chunk is processed by sequential reduce by key
  5. When all threads complete, we combine result of the incomplete chunks (carries) with a successor tile's partial result

The issue is related to the final step. Successor tile's partial result is written into values_result, which is fine. But we are trying to read value_results to combine it with carries[i], which is an issue since values_result is an output iterator.

We'd probably have to store partial result for the first key in the chunk in a temporary storage as well.

@jrhemstad jrhemstad added the thrust For all items related to Thrust. label Feb 22, 2023
@github-project-automation github-project-automation bot moved this to Todo in CCCL Nov 8, 2023
@jarmak-nv jarmak-nv transferred this issue from NVIDIA/thrust Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
thrust For all items related to Thrust.
Projects
Status: Todo
Development

No branches or pull requests

3 participants