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.

Sign up for GitHub

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

Jump to bottom

[FEA]: Use P2322R6 to determine accumulator type in Thrust #153

Open
1 of 4 tasks
gevtushenko opened this issue Jun 30, 2023 · 0 comments
Open
1 of 4 tasks

[FEA]: Use P2322R6 to determine accumulator type in Thrust #153

gevtushenko opened this issue Jun 30, 2023 · 0 comments
Labels
feature request New feature or request. thrust For all items related to Thrust.

Comments

@gevtushenko
Copy link
Collaborator

gevtushenko commented Jun 30, 2023 •
edited by jrhemstad
Loading

Is this a duplicate?

Area

Thrust

Is your feature request related to a problem? Please describe.

We adopted P2322R6 scheme in CUB (NVIDIA/cub#428), but Thrust still uses accumulator types that are surprising to users and do not follow general C++ rules like integer promotion.

Describe the solution you'd like

We should decide if we'd like to proceed with P2322R6 adoption in Thrust and follow CUB scheme of selecting accumulator types.

Tasks

  • Decide if the silent change of accumulator type is breaking
  • Prepare a list of standard algorithms that specify accumulator types
  • Consider preparing a paper to adopt P2322R6 to standard algorithms

CC: @jrhemstad @dkolsen-pgi

Describe alternatives you've considered

No response

Additional context

No response
@github-project-automation github-project-automation bot moved this to Todo in CCCL Jun 30, 2023
@isVoid isVoid mentioned this issue Jun 30, 2023
Merged
3 tasks
@miscco miscco added feature request New feature or request. thrust For all items related to Thrust. labels Jul 12, 2023
rapids-bot bot pushed a commit to rapidsai/cuspatial that referenced this issue Jul 28, 2023
@isVoid
Fix overflowing in intersection_intermediates.remove_if (#1209)
e4f5ac0 
EDIT: the thrust bug is a known issue. Tracked by NVIDIA/cccl#153

This PR fixes an issue in `remove_if`. Strangely, when calling `reduce_by_key` with iterator to integer,
and when using `thrust::plus<index_t>()`, even if by definition the argument type of the plus operator is strongly typed
with a higher bit  width integer type, and I expected that the flags (`uint8_t`) were cast to the higher
bit depth before addition, the overflow still happens. I have filed a thread in the thrust channel to discuss if this
is a bug in thrust.

Meanwhile, a quick WAR is to explicitly use a transform iterator to cast the uchar in to `index_t` before adding.
This should give the correct result.

Fixes #1200 
Depend on #1207

Authors:
  - Michael Wang (https://github.com/isVoid)
  - H. Thomson Comer (https://github.com/thomcom)

Approvers:
  - Mark Harris (https://github.com/harrism)
  - H. Thomson Comer (https://github.com/thomcom)

URL: #1209
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature request New feature or request. thrust For all items related to Thrust.
Projects
CCCL
Status: Todo
Milestone
No milestone
Development

No branches or pull requests
2 participants
@miscco @gevtushenko