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

Add radix_sort #712

Merged
merged 7 commits into from Jun 15, 2023
Merged

Add radix_sort #712

merged 7 commits into from Jun 15, 2023

Conversation

0382
Copy link
Contributor

@0382 0382 commented May 7, 2023

For build-in types, radix_sort is faster in most cases than compare based sort algorithm.

Copy link
Member

@jvdp1 jvdp1 left a comment

Choose a reason for hiding this comment

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

Thank you for this proposal. Here are some first general comments.

example/sorting/example_radix_sort.f90 Outdated Show resolved Hide resolved
example/sorting/example_radix_sort.f90 Outdated Show resolved Hide resolved
src/stdlib_sorting.fypp Show resolved Hide resolved
Comment on lines +438 to +470
pure module subroutine int8_radix_sort(array, reverse)
integer(kind=int8), dimension(:), intent(inout) :: array
logical, intent(in), optional :: reverse
end subroutine int8_radix_sort

pure module subroutine int16_radix_sort(array, work, reverse)
integer(kind=int16), dimension(:), intent(inout) :: array
integer(kind=int16), dimension(:), intent(inout), target, optional :: work
logical, intent(in), optional :: reverse
end subroutine int16_radix_sort

pure module subroutine int32_radix_sort(array, work, reverse)
integer(kind=int32), dimension(:), intent(inout) :: array
integer(kind=int32), dimension(:), intent(inout), target, optional :: work
logical, intent(in), optional :: reverse
end subroutine int32_radix_sort

pure module subroutine int64_radix_sort(array, work, reverse)
integer(kind=int64), dimension(:), intent(inout) :: array
integer(kind=int64), dimension(:), intent(inout), target, optional :: work
logical, intent(in), optional :: reverse
end subroutine int64_radix_sort

module subroutine sp_radix_sort(array, work, reverse)
real(kind=sp), dimension(:), intent(inout), target :: array
real(kind=sp), dimension(:), intent(inout), target, optional :: work
logical, intent(in), optional :: reverse
end subroutine sp_radix_sort

module subroutine dp_radix_sort(array, work, reverse)
real(kind=dp), dimension(:), intent(inout), target :: array
real(kind=dp), dimension(:), intent(inout), target, optional :: work
logical, intent(in), optional :: reverse
Copy link
Member

Choose a reason for hiding this comment

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

Could be part of a fypp loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could be part of a fypp loop.

stdlib defines xdp, qp, but no int128. While radix sort need int128 to support such width data type. Considering the difference between int8_radix_sort and other integer types, and integer array do not need target attribute. I think the current version is just ok.

test/sorting/test_sorting.f90 Show resolved Hide resolved
example/sorting/example_radix_sort.f90 Outdated Show resolved Hide resolved
src/stdlib_sorting.fypp Outdated Show resolved Hide resolved
test/sorting/test_sorting.f90 Outdated Show resolved Hide resolved
0382 and others added 2 commits May 7, 2023 19:59
Co-authored-by: Jeremie Vandenplas <jeremie.vandenplas@gmail.com>
@0382
Copy link
Contributor Author

0382 commented May 7, 2023

Thank you for this proposal. Here are some first general comments.

Thank you for review. I have updated the code according to your comments. Here are the changes:

  • shorten and clarify examples.
  • add test for real numbers.
  • change int64 to int_size for index integer type. (same as other sort subroutines)
  • add some comments for the algorithm code.
  • move the place of public radix_sort. (to agree with other sort interfaces)

@0382 0382 requested a review from jvdp1 May 7, 2023 13:59
@jvdp1
Copy link
Member

jvdp1 commented May 14, 2023

The specs file must also be updated (similar text to what is in the fypp file).

0382 and others added 2 commits May 17, 2023 20:18
Co-authored-by: Jeremie Vandenplas <jeremie.vandenplas@gmail.com>
@0382 0382 requested a review from jvdp1 May 18, 2023 09:05
Copy link
Member

@jvdp1 jvdp1 left a comment

Choose a reason for hiding this comment

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

LGTM. Pending minor comments, it seems almost ready to be merged.

doc/specs/stdlib_sorting.md Outdated Show resolved Hide resolved
doc/specs/stdlib_sorting.md Outdated Show resolved Hide resolved
doc/specs/stdlib_sorting.md Outdated Show resolved Hide resolved
doc/specs/stdlib_sorting.md Outdated Show resolved Hide resolved
doc/specs/stdlib_sorting.md Show resolved Hide resolved
example/sorting/example_radix_sort.f90 Outdated Show resolved Hide resolved
doc/specs/stdlib_sorting.md Outdated Show resolved Hide resolved
doc/specs/stdlib_sorting.md Outdated Show resolved Hide resolved
0382 and others added 2 commits May 21, 2023 10:07
Co-authored-by: Jeremie Vandenplas <jeremie.vandenplas@gmail.com>
Co-authored-by: 0382 <18322825326@163.com>
Copy link
Member

@jvdp1 jvdp1 left a comment

Choose a reason for hiding this comment

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

Thank you @0382. LGTM. I'll merge it.

@jvdp1 jvdp1 merged commit 7cdecb5 into fortran-lang:master Jun 15, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants