Add opt-in GPU radix sort via sort alg keyword - #90
Merged
Conversation
Member
|
KA has |
Member
Author
Yea.. I checked it now. Thanks for giving up heads up. I will change the commit. |
shreyas-omkar
force-pushed
the
sh/sort-optim
branch
2 times, most recently
from
June 24, 2026 10:50
b5f4620 to
270e578
Compare
shreyas-omkar
marked this pull request as ready for review
June 27, 2026 18:41
Exclusive ScanPrefixes scans should keep each block's local exclusive output and store the full block aggregate separately for the inter-block prefix pass. Add a non-uniform multi-block regression test that catches the all-ones masking case. Co-authored-by: shreyas-omkar <shreyashegdeplus06@gmail.com>
Bring over the PR JuliaGPU#90 sort changes without the ScanPrefixes fix: GPU sortperm now defaults to merge_sortperm!, merge_sort! hoists non-identity by= transforms, and radix_sort!/radix_sort are available as direct APIs with correctness tests and a benchmark driver. The radix path remains opt-in at this point; sort! still defaults to merge sort. Co-authored-by: shreyas-omkar <shreyashegdeplus06@gmail.com>
Add SortAlgorithm marker types and route sort!/sort/sortperm!/sortperm through alg=. RadixSort is reachable only through the public sort API, while MergeSort(lowmem=true) selects the existing low-memory permutation path. Keep the default behavior unchanged: GPU sort uses merge sort, GPU sortperm uses merge_sortperm!, and CPU sort uses sample sort.
maleadt
changed the title
sortperm!
Member
|
I squashed into three logical commits. LGTM for an initial version; let's merge this a bit more rapidly rather than dragging the PR on like with mapreduce. |
Member
Author
|
Sure @maleadt, I'll try and get done with this PR ASAP. |
Member
|
Seems like I made a mistake here, let me investigate. EDIT: turns out my added tests exposes an issue with the DecoupledLoopback implementation. |
Member
|
That latest commit closes #84. If we're squashing i think we should split that one up into a separate PR |
Member
|
I was going to merge; hence cleaning up the history here first. |
… test The non-uniform exclusive accumulate test exposes block-carry bugs that all-ones data masks. ScanPrefixes (the default algorithm) is correct here, but DecoupledLookback's exclusive multi-block carries are still wrong on non-uniform data, so it is excluded from this assertion and tracked as a separate follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds an opt-in GPU radix sort implementation and makes it reachable through the public sort API:
The radix path supports UInt32, Int32, Float32, UInt64, Int64, and Float64. Unsupported element types or custom lt / by settings fall back to merge sort.
The default behavior is unchanged: GPU sort! still uses merge sort unless alg=AK.RadixSort() is passed explicitly.
Details
Closes #84