Skip to content
This repository was archived by the owner on Mar 21, 2024. It is now read-only.

Use CUB version of merge sort #1511

Merged

Conversation

gevtushenko
Copy link
Collaborator

This PR removes the Thrust version of merge sort and uses CUB one instead.

@gevtushenko gevtushenko requested a review from alliepiper August 26, 2021 15:18
@gevtushenko
Copy link
Collaborator Author

run tests

@gevtushenko gevtushenko force-pushed the main-feature/github/use_cub_merge_sort branch from f00fabe to a212bb6 Compare August 26, 2021 15:23
@gevtushenko
Copy link
Collaborator Author

run tests

@gevtushenko gevtushenko force-pushed the main-feature/github/use_cub_merge_sort branch from a212bb6 to 6cc34dc Compare August 26, 2021 16:39
@gevtushenko
Copy link
Collaborator Author

run tests

@gevtushenko
Copy link
Collaborator Author

DVS: 30345552

@gevtushenko gevtushenko force-pushed the main-feature/github/use_cub_merge_sort branch from 6cc34dc to 83eaecc Compare August 27, 2021 15:15
@gevtushenko
Copy link
Collaborator Author

run tests

@alliepiper alliepiper self-assigned this Sep 21, 2021
@alliepiper alliepiper added P1: should have Necessary, but not critical. testing: gpuCI passed Passed gpuCI testing. testing: internal ci in progress Currently testing on internal NVIDIA CI (DVS). labels Sep 21, 2021
@alliepiper alliepiper added this to the 1.15.0 milestone Sep 21, 2021
@alliepiper alliepiper added P2: nice to have Desired, but not necessary. and removed P1: should have Necessary, but not critical. labels Oct 14, 2021
@alliepiper
Copy link
Collaborator

@senior-zero Let's try to get this in for 1.15 if DVS is passing and it's ready to merge, but this is super low priority if it's going to take actual effort.

Copy link
Collaborator

@alliepiper alliepiper left a comment

Choose a reason for hiding this comment

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

LGTM


for (int pass = 0; pass < num_passes; ++pass, ping = !ping)
if (STABLE::value)
Copy link
Collaborator

Choose a reason for hiding this comment

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

These may need to be THRUST_IF_CONSTEXPR if tests complain about the constexpr arg.

@alliepiper alliepiper assigned gevtushenko and unassigned alliepiper Oct 14, 2021
@gevtushenko
Copy link
Collaborator Author

run tests

@gevtushenko gevtushenko force-pushed the main-feature/github/use_cub_merge_sort branch from 13707fb to 5a17e55 Compare October 15, 2021 13:18
@gevtushenko
Copy link
Collaborator Author

run tests

@gevtushenko gevtushenko force-pushed the main-feature/github/use_cub_merge_sort branch from 5a17e55 to f7e39a8 Compare October 15, 2021 13:37
@gevtushenko
Copy link
Collaborator Author

run tests

@gevtushenko
Copy link
Collaborator Author

DVS CL: 30537652

@gevtushenko gevtushenko force-pushed the main-feature/github/use_cub_merge_sort branch 2 times, most recently from b16eae9 to ce4c107 Compare October 15, 2021 15:31
@gevtushenko
Copy link
Collaborator Author

run tests

@alliepiper
Copy link
Collaborator

Hoping DVS behaves this time 😅

DVS CL: 30549730

@alliepiper
Copy link
Collaborator

The DVS builds are all timing out with this update, and gpuCI seems to be taking about an hour longer than before. We should track down why and fix this before merging.

Moving to 1.16 since this isn't urgent for 1.15.

@alliepiper alliepiper modified the milestones: 1.15.0, 1.16.0 Oct 20, 2021
Copy link
Collaborator

@alliepiper alliepiper left a comment

Choose a reason for hiding this comment

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

Need to track down compile time increases between old and new merge sort implementations.

@gevtushenko gevtushenko force-pushed the main-feature/github/use_cub_merge_sort branch from ce4c107 to 9dd2f22 Compare October 22, 2021 00:16
@gevtushenko
Copy link
Collaborator Author

run tests

@gevtushenko gevtushenko force-pushed the main-feature/github/use_cub_merge_sort branch from 9dd2f22 to 6192d9a Compare October 22, 2021 10:29
@gevtushenko
Copy link
Collaborator Author

run tests

@gevtushenko gevtushenko force-pushed the main-feature/github/use_cub_merge_sort branch from 6192d9a to 91d5e6c Compare October 22, 2021 19:07
@gevtushenko gevtushenko merged commit f828d38 into NVIDIA:main Oct 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P2: nice to have Desired, but not necessary. testing: gpuCI passed Passed gpuCI testing. testing: internal ci in progress Currently testing on internal NVIDIA CI (DVS).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants