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

Fix overflowing in intersection_intermediates.remove_if #1209

Merged
merged 15 commits into from
Jul 28, 2023

Conversation

isVoid
Copy link
Contributor

@isVoid isVoid commented Jun 29, 2023

Description

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

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@github-actions github-actions bot added the libcuspatial Relates to the cuSpatial C++ library label Jun 29, 2023
@isVoid isVoid added bug Something isn't working non-breaking Non-breaking change labels Jun 30, 2023
@isVoid isVoid marked this pull request as ready for review July 25, 2023 16:17
@isVoid isVoid requested a review from a team as a code owner July 25, 2023 16:17
@isVoid isVoid requested review from trxcllnt and harrism July 25, 2023 16:17
@isVoid isVoid self-assigned this Jul 27, 2023
@thomcom
Copy link
Contributor

thomcom commented Jul 28, 2023

/merge

@rapids-bot rapids-bot bot merged commit e4f5ac0 into rapidsai:branch-23.08 Jul 28, 2023
45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working libcuspatial Relates to the cuSpatial C++ library non-breaking Non-breaking change
Projects
Status: Review
Development

Successfully merging this pull request may close these issues.

[BUG]: Bad results returned from pairwise_linestring_intersection on large self-equal LineStrings.
3 participants