Sort NaNs to the end for descending order by antonwolfy · Pull Request #3066 · IntelPython/dpnp

antonwolfy · 2026-09-10T15:35:07Z

dpnp.sort, dpnp.argsort, and their dpnp.ndarray/dpnp.tensor counterparts placed NaN values at the beginning when sorting in descending order, while NumPy 2.5 keeps NaN values 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:

Merge-sort comparators: only the comparison between non-NaN values is reversed for descending order, so NaNs stay at the end. For complex values the NaN-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.
Radix-sort float casts (half / float32 / float64): map NaN to 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_k inherits the change through the shared descending comparator: NaN values (and complex values with a NaN component) are now treated as the smallest, so mode="largest" no longer returns them ahead of finite values.

NaN ordering under sort/argsort is left implementation-defined by the Python Array API standard, and top_k explicitly permits sorting NaN values to either end. The new "NaN last" behavior is therefore spec-conforming and is chosen to match NumPy 2.5; it is now consistent across the dpnp and dpnp.tensor namespaces.

Have you provided a meaningful PR description?
Have you added a test, reproducer or referred to an issue with a reproducer?
Have you tested your changes locally for CPU and GPU devices?
Have you made sure that new changes do not introduce compiler warnings?
Have you checked performance impact of proposed changes?
Have you added documentation for your changes, if necessary?
Have you added your changes to the changelog?

`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.

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.

github-actions · 2026-09-10T16:44:19Z

View rendered docs @ https://intelpython.github.io/dpnp/pull/3066/index.html

github-actions · 2026-09-10T17:21:09Z

Array API standard conformance tests for dpnp=0.21.0dev8=py314ha0e2e8e_20 ran successfully.
Passed: 1376
Failed: 0
Skipped: 6

coveralls · 2026-09-10T17:38:11Z

coverage: 78.477%. remained the same — fix-descending-nan into master

vlad-perevezentsev · 2026-09-11T11:27:38Z

            )
        )

-    # thread_unsafe marker requires pytest-run-parallel, not used by dpnp


Why was this removed?

Reducing the number of lines in the diff against the upstream.
Commenting below line is clear message we don't support that, no need for the clarification.

vlad-perevezentsev · 2026-09-11T11:31:18Z

+            kwargs = {"kind": "stable"}
+        else:
+            # numpy rejects `kind` combined with `descending`; `stable=True`
+            # is its replacement for forcing determinism. cupy's argsort is


dpnp.argsort does accept stable

In [10]: dpnp.argsort(a, descending=True, stable=True) Out[10]: array([5, 4, 3, 2, 1, 0])

This comment is copied verbatim from the upstream cupy test (tests/cupy_tests/sorting_tests/test_sort.py); it's accurate for cupy, whose argsort is always stable and doesn't take a stable kwarg. I'd prefer to keep it unchanged to minimize the diff against the upstream test suite — the if xp is numpy: guard already ensures stable=True is passed only to numpy, and dpnp's default-stable argsort matches.

vlad-perevezentsev · 2026-09-11T11:36:47Z

@@ -106,10 +107,38 @@ struct ExtendedComplexFPLess
 template <typename cT>
 struct ExtendedComplexFPGreater


ExtendedComplexFPGreater line by line copy of ExtendedComplexFPLess with operands swapped
Probably easier to use something like return ExtendedComplexFPLess<cT>{}(-v1, -v2); below

Good suggestion, done — ExtendedComplexFPGreater now delegates to ExtendedComplexFPLess<cT>{}(-v1, -v2). Negation preserves NaN-ness, so the NaN grouping (and its ordering to the end) is unchanged while the finite comparison is reversed.

One perf note for the record: this form constructs two std::complex temporaries (4 float negations) per comparison, whereas the previous hand-written body compared existing references. It's safe to ignore in practice — complex sort goes through the merge path (complex isn't radix-eligible), the per-compare cost is dominated by the branchy NaN-group logic inside ExtendedComplexFPLess, the negations are trivial and typically optimized out, and complex isn't a hot dtype for large sorts. The readability win of dropping the duplicated ~30-line body outweighs the negligible cost; we can revisit if profiling ever flags complex descending sort.

The entry now states the new behavior (NaN values placed last instead of first) rather than reading as if they are placed first.

NaN values are ordered to the end for both modes, so describing them as "the smallest" was inaccurate for mode="smallest". Scope the changelog entry to mode="largest" (the only behavior that changed) and state in the top_k docstring that NaNs are ordered last for both modes.

Add the same NaN-ordering note to the `dpnp.ndarray.sort` and `dpnp.ndarray.argsort` docstrings that the module-level `dpnp.sort` and `dpnp.argsort` already carry.

Parametrize `test_descending_nan` over `kind` so the descending NaN-at-end behavior is validated for the radix-sort path as well as merge sort.

Implement `ExtendedComplexFPGreater` as `ExtendedComplexFPLess{}(-v1, -v2)` instead of duplicating the comparison logic with operands swapped. Negation preserves NaN-ness, so the NaN-based grouping (and its ordering to the end) is unchanged while the finite-component comparison is reversed.

antonwolfy added 4 commits September 10, 2026 15:52

Port CuPy descending sort/argsort tests

6872854

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`).

Add PR reference to the changelog entry

8383540

antonwolfy added this to the 0.21.0 release milestone Sep 10, 2026

antonwolfy self-assigned this Sep 10, 2026

antonwolfy requested review from ndgrigorian and vlad-perevezentsev as code owners September 10, 2026 15:35

antonwolfy added 3 commits September 10, 2026 17:38

Document NaN ordering for descending sort/argsort

7e8cadf

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.

Cover descending complex NaN ordering in dpnp.tensor sort test

ce73016

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.

vlad-perevezentsev reviewed Sep 11, 2026

View reviewed changes

antonwolfy added 5 commits September 11, 2026 14:46

Clarify changelog wording for descending NaN ordering

68af50f

The entry now states the new behavior (NaN values placed last instead of first) rather than reading as if they are placed first.

Document NaN ordering on dpnp.ndarray sort/argsort methods

808365e

Add the same NaN-ordering note to the `dpnp.ndarray.sort` and `dpnp.ndarray.argsort` docstrings that the module-level `dpnp.sort` and `dpnp.argsort` already carry.

Exercise all sort kinds in descending NaN float test

490c858

Parametrize `test_descending_nan` over `kind` so the descending NaN-at-end behavior is validated for the radix-sort path as well as merge sort.

Navigation Menu

Uh oh!

Sort NaNs to the end for descending order - #3066

Sort NaNs to the end for descending order#3066
antonwolfy wants to merge 12 commits into
masterfrom
fix-descending-nan

antonwolfy commented Sep 10, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 10, 2026 •

edited

Loading

Uh oh!

coveralls commented Sep 10, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vlad-perevezentsev Sep 11, 2026

Uh oh!

antonwolfy Sep 11, 2026

Uh oh!

vlad-perevezentsev Sep 11, 2026

Uh oh!

antonwolfy Sep 11, 2026 •

edited

Loading

Uh oh!

vlad-perevezentsev Sep 11, 2026

Uh oh!

antonwolfy Sep 11, 2026 •

edited

Loading

Uh oh!

antonwolfy Sep 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -106,10 +107,38 @@ struct ExtendedComplexFPLess
		template <typename cT>
		struct ExtendedComplexFPGreater

Uh oh!

Conversation

antonwolfy commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vlad-perevezentsev Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

antonwolfy Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

vlad-perevezentsev Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

antonwolfy Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vlad-perevezentsev Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

antonwolfy Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

antonwolfy Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

antonwolfy commented Sep 10, 2026 •

edited

Loading

github-actions Bot commented Sep 10, 2026 •

edited

Loading

coveralls commented Sep 10, 2026 •

edited

Loading

antonwolfy Sep 11, 2026 •

edited

Loading

antonwolfy Sep 11, 2026 •

edited

Loading