Sort NaNs to the end for descending order - #3066
Open
antonwolfy wants to merge 7 commits into
Open
Conversation
`dpnp.sort`/`dpnp.argsort` (and their `dpnp.ndarray`/`dpnp.tensor` counterparts) placed `NaN` values first when sorting in descending order, while NumPy 2.5 keeps `NaN` at the end for both ascending and descending order. Fix the merge-sort comparators so that only the comparison between non-NaN values is reversed for descending order, and make the radix-sort float casts map `NaN` to the maximum key so it lands in the last bucket regardless of direction.
Bring in the descending-order sort/argsort test coverage from cupy#10088, adapted to dpnp's signature (`order` remains a positional parameter, so the keyword-only checks pass an explicit `order=None`).
Backport NumPy 2.5 descending-order test coverage (numpy gh-31345, gh-31476, gh-31557) into dpnp's own sort test suite, adapted to the dtypes dpnp supports: * NaNs sort to the end for both ascending and descending float sorts. * NaN-containing complex values sort to the end in the same groups for both orders, with finite values kept in lexicographic order. * a stable (arg)sort keeps the original relative order of equal elements in both directions. Expected results are delegated to `numpy.sort`/`numpy.argsort` with `stable=True, descending=...`, so the tests require NumPy >= 2.5.
antonwolfy
requested review from
ndgrigorian and
vlad-perevezentsev
as code owners
September 10, 2026 15:35
Note in the `dpnp` and `dpnp.tensor` sort/argsort docstrings that NaN values (and complex values with a NaN component) are ordered to the end regardless of the `descending` flag.
The descending sort fix routes float and complex `top_k(mode="largest")` through the same comparator, so NaN values (and complex values with a NaN component) are now treated as the smallest and no longer surface ahead of finite values. Add a `dpnp.tensor.top_k` NaN test, document the behavior in the `top_k` docstring, and add a changelog note.
Extend `test_sort_complex_fp_nan` to also sort with `descending=True` and compare against NumPy, so the dpnp.tensor layer exercises the complex NaN-at-end ordering for both directions. Guarded on numpy>=2.5.
Contributor
|
View rendered docs @ https://intelpython.github.io/dpnp/pull/3066/index.html |
Contributor
|
Array API standard conformance tests for dpnp=0.21.0dev8=py314ha0e2e8e_15 ran successfully. |
Collaborator
|
coverage: 78.477%. remained the same — fix-descending-nan into master |
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
2 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.
dpnp.sort,dpnp.argsort, and theirdpnp.ndarray/dpnp.tensorcounterparts placedNaNvalues at the beginning when sorting in descending order, while NumPy 2.5 keepsNaNvalues at the end for both ascending and descending order.For descending order the sort kernels simply reversed the ascending comparison, which also moved
NaN(treated as the largest value) to the front.The PR proposes to change:
NaNvalues is reversed for descending order, soNaNs stay at the end. For complex values theNaN-ness groups(no nan) -> (imag nan) -> (real nan) -> (all nan)keep the same trailing order as ascending, and only the finite-component lexicographic comparison is reversed.NaNto the maximum key, so it lands in the last bucket regardless of direction. This keeps the descending float radix fast path intact rather than falling back to a slower comparator.dpnp.tensor.top_kinherits the change through the shared descending comparator:NaNvalues (and complex values with aNaNcomponent) are now treated as the smallest, somode="largest"no longer returns them ahead of finite values.NaNordering undersort/argsortis left implementation-defined by the Python Array API standard, andtop_kexplicitly permits sortingNaNvalues to either end. The new "NaNlast" behavior is therefore spec-conforming and is chosen to match NumPy 2.5; it is now consistent across thedpnpanddpnp.tensornamespaces.