Implement unique function to stdlib_sorting - #1200
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1200 +/- ##
==========================================
- Coverage 68.81% 68.20% -0.61%
==========================================
Files 408 19 -389
Lines 13726 2378 -11348
Branches 1552 0 -1552
==========================================
- Hits 9446 1622 -7824
+ Misses 4280 756 -3524 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (11)
src/sorting/stdlib_sorting.fypp:536
- This example uses
b = unique(...)as ifuniquewere a function, butuniqueis a subroutine in this module. The example as written won’t compile.
!! b = unique(a, .true.)
src/sorting/stdlib_sorting.fypp:489
- The module-level documentation describes
uniqueas a function (output = unique(...)), but the public API implemented below is a generic subroutine (call unique(array, sorted_output, output[, tolerance])). This mismatch will mislead users and contradicts the rest of the module’s procedure docs.
!! The generic function implementing the `UNIQUE` algorithm to return the
!! distinct elements of a rank-1 array. The result may either preserve the
!! order of first occurrence or be returned in sorted order.
!!
!! Its use has the syntax:
src/sorting/stdlib_sorting.fypp:526
- This example uses
b = unique(...)as ifuniquewere a function, butuniqueis a subroutine in this module. The example as written won’t compile.
This issue also appears on line 536 of the same file.
!! b = unique(a, .false.)
src/sorting/stdlib_sorting.fypp:516
- The inline markup for
real(xdp)is malformed (real(xdp)), which breaks the rendered documentation.
!! **Note:** The unsorted (`sorted_output = .false.`) implementation is
!! currently unavailable for `real(`xdp`)` because hashing is
!! performed on the underlying binary representation, which is not
!! sufficiently portable for this kind.
src/sorting/stdlib_sorting_unique.fypp:53
- For real overloads,
toleranceis validated unconditionally, even whensorted_outputis.false.(where tolerance is documented as not applicable). This means callers can get a tolerance-relatederror stopin unsorted mode, and passing a positive tolerance is silently ignored. Consider only reading/validatingtolerancewhensorted_outputis.true., and rejectingpresent(tolerance)whensorted_outputis.false..
#:if t1.startswith('real')
tolerance_ = optval(tolerance, 0.0_${name1}$)
if(tolerance_ < 0.0_${name1}$) error stop "tolerance must be non-negative"
#:endif
allocate(temp, source=A)
src/sorting/stdlib_sorting_unique.fypp:11
use stdlib_constantsis unused in this submodule (all referenced symbols come from host association / intrinsics). Keeping it forces an extra library dependency in the sorting CMake target.
use stdlib_constants
src/sorting/stdlib_sorting_unique.fypp:93
outputis allocated tosize(temp)and then immediately assignedpack(temp, mask), whose size is typically smaller. With allocatable assignment, this triggers a deallocate/reallocate anyway, so the manual allocation block is redundant and can add overhead.
if (.not. allocated(output)) then
allocate(output(size(temp)))
else if (size(output) < size(temp)) then
deallocate(output)
allocate(output(size(temp)))
src/sorting/stdlib_sorting_unique.fypp:130
- Same as in
*_sort_unique:outputis allocated tosize(temp)and then assignedpack(temp, mask), which will generally force reallocation. This allocation block can be removed to avoid extra allocate/deallocate churn.
if (.not. allocated(output)) then
allocate(output(size(temp)))
else if (size(output) < size(temp)) then
deallocate(output)
allocate(output(size(temp)))
doc/specs/stdlib_sorting.md:30
- The overview still says there are "four overloaded subroutines", but the list now contains five (
ORD_SORT,SORT,RADIX_SORT,SORT_INDEX,UNIQUE).
The module `stdlib_sorting` defines several public entities, two
default integer parameters, `int_index` and `int_index_low`, and four overloaded
subroutines: `ORD_SORT`, `SORT`, `RADIX_SORT`, `SORT_INDEX` and `UNIQUE`.
The overloaded subroutines also each have several specific names for
test/sorting/test_sorting_unique.fypp:128
- The real-kind tests exclude
xdpentirely (#:if name1 != 'xdp'). Since the implementation only lacks the unsorted mode forxdp, this leaves the sorteduniquepath untested whenWITH_XDPis enabled.
#:for t1, t2, name1, cpp1 in REAL_TYPES_ALT_NAME
#:if name1 != 'xdp'
block
src/sorting/stdlib_sorting.fypp:491
- The documented syntax line still shows the function form (
output = unique(...)), but the implemented interface iscall unique(array, sorted_output, output[, tolerance]).
!! Its use has the syntax:
!! output = unique(array, sorted_output[, tolerance] )
!!
Co-authored-by: José Alves <102541118+jalvesz@users.noreply.github.com>
This PR adds a generic
uniquefunction tostdlib_sortingfor extracting the distinct elements of rank-1 arrays.sorted_output = .true.)sorted_output = .false.)toleranceargument for real arrays whensorted_output = .true.to support approximate equality comparisons.Limitations
sorted_output = .true..real(xdp)is currently unavailable because hashing relies on the underlying binary representation.Solves Implement a
uniquefunction returning only the unique values in a vector. #940.