From 66c48ade09805fab4fb48068929964b4085dab6b Mon Sep 17 00:00:00 2001 From: Angus Hollands Date: Thu, 12 Jan 2023 17:21:37 +0000 Subject: [PATCH 1/3] fix: be more permissive with sort translation --- src/awkward/operations/ak_argsort.py | 4 ++-- src/awkward/operations/ak_sort.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/awkward/operations/ak_argsort.py b/src/awkward/operations/ak_argsort.py index 8532ee1fb3..98d6c264b2 100644 --- a/src/awkward/operations/ak_argsort.py +++ b/src/awkward/operations/ak_argsort.py @@ -70,9 +70,9 @@ def _impl(array, axis, ascending, stable, highlevel, behavior): def _nep_18_impl(a, axis=-1, kind=None, order=unsupported): if kind is None: stable = False - elif kind == "stable": + elif kind == "stable" or kind == "mergesort": stable = True - elif kind == "heapsort": + elif kind == "heapsort" or kind == "quicksort": stable = False else: raise ak._errors.wrap_error( diff --git a/src/awkward/operations/ak_sort.py b/src/awkward/operations/ak_sort.py index 8ea153c800..a35b7eba1e 100644 --- a/src/awkward/operations/ak_sort.py +++ b/src/awkward/operations/ak_sort.py @@ -57,9 +57,9 @@ def _impl(array, axis, ascending, stable, highlevel, behavior): def _nep_18_impl(a, axis=-1, kind=None, order=unsupported): if kind is None: stable = False - elif kind == "stable": + elif kind == "stable" or kind == "mergesort": stable = True - elif kind == "heapsort": + elif kind == "heapsort" or kind == "quicksort": stable = False else: raise ak._errors.wrap_error( From 09a846fff5c85fb6601de985653653b667175fda Mon Sep 17 00:00:00 2001 From: Angus Hollands Date: Thu, 12 Jan 2023 17:21:44 +0000 Subject: [PATCH 2/3] chore: remove quicksort --- .../src/cpu-kernels/awkward_quick_sort.cpp | 333 ------------------ kernel-specification.yml | 118 ------- 2 files changed, 451 deletions(-) delete mode 100644 awkward-cpp/src/cpu-kernels/awkward_quick_sort.cpp diff --git a/awkward-cpp/src/cpu-kernels/awkward_quick_sort.cpp b/awkward-cpp/src/cpu-kernels/awkward_quick_sort.cpp deleted file mode 100644 index 475813a71e..0000000000 --- a/awkward-cpp/src/cpu-kernels/awkward_quick_sort.cpp +++ /dev/null @@ -1,333 +0,0 @@ -// BSD 3-Clause License; see https://github.com/scikit-hep/awkward-1.0/blob/main/LICENSE - -#define FILENAME(line) FILENAME_FOR_EXCEPTIONS_C("src/cpu-kernels/awkward_quick_sort.cpp", line) - -#include "awkward/kernels.h" - -template -bool order_ascending(T left, T right) -{ - return left <= right; -} - -template -bool order_descending(T left, T right) -{ - return left >= right; -} - -template -bool binary_op(T left, T right, bool (*f)(T, T)) { - return (*f)(left, right); -} - -template -int quick_sort(T *arr, - int64_t elements, - int64_t* beg, - int64_t* end, - int64_t maxlevels, - P& predicate) { - int64_t low = 0; - int64_t high = 0; - int64_t i = 0; - beg[0] = 0; - end[0] = elements; - while (i >= 0) { - low = beg[i]; - high = end[i]; - if (high - low > 1) { - int64_t mid = low + ((high - low) >> 1); - T pivot = arr[mid]; - arr[mid] = arr[low]; - - if (i == maxlevels - 1) { - return -1; - } - high--; - while (low < high) { - while (binary_op(pivot, arr[high], predicate) && low < high) { - high--; - } - if (low < high) { - arr[low++] = arr[high]; - } - while (binary_op(arr[low], pivot, predicate) && low < high) { - low++; - } - if (low < high) { - arr[high--] = arr[low]; - } - } - arr[low] = pivot; - mid = low + 1; - while (low > beg[i] && arr[low - 1] == pivot) { - low--; - } - while (mid < end[i] && arr[mid] == pivot) { - mid++; - } - if (low - beg[i] > end[i] - mid) { - beg[i + 1] = mid; - end[i + 1] = end[i]; - end[i++] = low; - } else { - beg[i + 1] = beg[i]; - end[i + 1] = low; - beg[i++] = mid; - } - } else { - i--; - } - } - return 0; -} - -template -ERROR awkward_quick_sort( - T* tmpptr, - int64_t* tmpbeg, - int64_t* tmpend, - const int64_t* fromstarts, - const int64_t* fromstops, - bool ascending, - int64_t length, - int64_t maxlevels) { - - if (ascending) { - for (int64_t i = 0; i < length; i++) { - if (quick_sort(&(tmpptr[fromstarts[i]]), - fromstops[i] - fromstarts[i], - tmpbeg, - tmpend, - maxlevels, - order_ascending) < 0) { - return failure("failed to sort an array", i, fromstarts[i], FILENAME(__LINE__)); - } - } - } - else { - for (int64_t i = 0; i < length; i++) { - if (quick_sort(&(tmpptr[fromstarts[i]]), - fromstops[i] - fromstarts[i], - tmpbeg, - tmpend, - maxlevels, - order_descending) < 0) { - return failure("failed to sort an array", i, fromstarts[i], FILENAME(__LINE__)); - } - } - } - - return success(); -} - -ERROR awkward_quick_sort_bool( - bool* tmpptr, - int64_t* tmpbeg, - int64_t* tmpend, - const int64_t* fromstarts, - const int64_t* fromstops, - bool ascending, - int64_t length, - int64_t maxlevels) { - return awkward_quick_sort( - tmpptr, - tmpbeg, - tmpend, - fromstarts, - fromstops, - ascending, - length, - maxlevels); -} -ERROR awkward_quick_sort_int8( - int8_t* tmpptr, - int64_t* tmpbeg, - int64_t* tmpend, - const int64_t* fromstarts, - const int64_t* fromstops, - bool ascending, - int64_t length, - int64_t maxlevels) { - return awkward_quick_sort( - tmpptr, - tmpbeg, - tmpend, - fromstarts, - fromstops, - ascending, - length, - maxlevels); -} -ERROR awkward_quick_sort_uint8( - uint8_t* tmpptr, - int64_t* tmpbeg, - int64_t* tmpend, - const int64_t* fromstarts, - const int64_t* fromstops, - bool ascending, - int64_t length, - int64_t maxlevels) { - return awkward_quick_sort( - tmpptr, - tmpbeg, - tmpend, - fromstarts, - fromstops, - ascending, - length, - maxlevels); -} -ERROR awkward_quick_sort_int16( - int16_t* tmpptr, - int64_t* tmpbeg, - int64_t* tmpend, - const int64_t* fromstarts, - const int64_t* fromstops, - bool ascending, - int64_t length, - int64_t maxlevels) { - return awkward_quick_sort( - tmpptr, - tmpbeg, - tmpend, - fromstarts, - fromstops, - ascending, - length, - maxlevels); -} -ERROR awkward_quick_sort_uint16( - uint16_t* tmpptr, - int64_t* tmpbeg, - int64_t* tmpend, - const int64_t* fromstarts, - const int64_t* fromstops, - bool ascending, - int64_t length, - int64_t maxlevels) { - return awkward_quick_sort( - tmpptr, - tmpbeg, - tmpend, - fromstarts, - fromstops, - ascending, - length, - maxlevels); -} -ERROR awkward_quick_sort_int32( - int32_t* tmpptr, - int64_t* tmpbeg, - int64_t* tmpend, - const int64_t* fromstarts, - const int64_t* fromstops, - bool ascending, - int64_t length, - int64_t maxlevels) { - return awkward_quick_sort( - tmpptr, - tmpbeg, - tmpend, - fromstarts, - fromstops, - ascending, - length, - maxlevels); -} -ERROR awkward_quick_sort_uint32( - uint32_t* tmpptr, - int64_t* tmpbeg, - int64_t* tmpend, - const int64_t* fromstarts, - const int64_t* fromstops, - bool ascending, - int64_t length, - int64_t maxlevels) { - return awkward_quick_sort( - tmpptr, - tmpbeg, - tmpend, - fromstarts, - fromstops, - ascending, - length, - maxlevels); -} -ERROR awkward_quick_sort_int64( - int64_t* tmpptr, - int64_t* tmpbeg, - int64_t* tmpend, - const int64_t* fromstarts, - const int64_t* fromstops, - bool ascending, - int64_t length, - int64_t maxlevels) { - return awkward_quick_sort( - tmpptr, - tmpbeg, - tmpend, - fromstarts, - fromstops, - ascending, - length, - maxlevels); -} -ERROR awkward_quick_sort_uint64( - uint64_t* tmpptr, - int64_t* tmpbeg, - int64_t* tmpend, - const int64_t* fromstarts, - const int64_t* fromstops, - bool ascending, - int64_t length, - int64_t maxlevels) { - return awkward_quick_sort( - tmpptr, - tmpbeg, - tmpend, - fromstarts, - fromstops, - ascending, - length, - maxlevels); -} -ERROR awkward_quick_sort_float32( - float* tmpptr, - int64_t* tmpbeg, - int64_t* tmpend, - const int64_t* fromstarts, - const int64_t* fromstops, - bool ascending, - int64_t length, - int64_t maxlevels) { - return awkward_quick_sort( - tmpptr, - tmpbeg, - tmpend, - fromstarts, - fromstops, - ascending, - length, - maxlevels); -} -ERROR awkward_quick_sort_float64( - double* tmpptr, - int64_t* tmpbeg, - int64_t* tmpend, - const int64_t* fromstarts, - const int64_t* fromstops, - bool ascending, - int64_t length, - int64_t maxlevels) { - return awkward_quick_sort( - tmpptr, - tmpbeg, - tmpend, - fromstarts, - fromstops, - ascending, - length, - maxlevels); -} diff --git a/kernel-specification.yml b/kernel-specification.yml index ac0537148b..8a95afb376 100644 --- a/kernel-specification.yml +++ b/kernel-specification.yml @@ -5681,124 +5681,6 @@ kernels: automatic-tests: true manual-tests: [] - - name: awkward_quick_sort - specializations: - - name: awkward_quick_sort_bool - args: - - {name: tmpptr, type: "List[bool]", dir: in, role: IndexedArray-index} - - {name: tmpbeg, type: "List[int64_t]", dir: in, role: IndexedArray-index} - - {name: tmpend, type: "List[int64_t]", dir: in, role: IndexedArray-index} - - {name: fromstarts, type: "Const[List[int64_t]]", dir: in, role: ListArray-starts} - - {name: fromstops, type: "Const[List[int64_t]]", dir: in, role: ListArray-stops} - - {name: ascending, type: "bool", dir: in, role: ListArray-replacement} - - {name: length, type: "int64_t", dir: in, role: default} - - {name: maxlevels, type: "int64_t", dir: in, role: default} - - name: awkward_quick_sort_int8 - args: - - {name: tmpptr, type: "List[int8_t]", dir: in, role: IndexedArray-index} - - {name: tmpbeg, type: "List[int64_t]", dir: in, role: IndexedArray-index} - - {name: tmpend, type: "List[int64_t]", dir: in, role: IndexedArray-index} - - {name: fromstarts, type: "Const[List[int64_t]]", dir: in, role: ListArray-starts} - - {name: fromstops, type: "Const[List[int64_t]]", dir: in, role: ListArray-stops} - - {name: ascending, type: "bool", dir: in, role: ListArray-replacement} - - {name: length, type: "int64_t", dir: in, role: default} - - {name: maxlevels, type: "int64_t", dir: in, role: default} - - name: awkward_quick_sort_int16 - args: - - {name: tmpptr, type: "List[int16_t]", dir: in, role: IndexedArray-index} - - {name: tmpbeg, type: "List[int64_t]", dir: in, role: IndexedArray-index} - - {name: tmpend, type: "List[int64_t]", dir: in, role: IndexedArray-index} - - {name: fromstarts, type: "Const[List[int64_t]]", dir: in, role: ListArray-starts} - - {name: fromstops, type: "Const[List[int64_t]]", dir: in, role: ListArray-stops} - - {name: ascending, type: "bool", dir: in, role: ListArray-replacement} - - {name: length, type: "int64_t", dir: in, role: default} - - {name: maxlevels, type: "int64_t", dir: in, role: default} - - name: awkward_quick_sort_int32 - args: - - {name: tmpptr, type: "List[int32_t]", dir: in, role: IndexedArray-index} - - {name: tmpbeg, type: "List[int64_t]", dir: in, role: IndexedArray-index} - - {name: tmpend, type: "List[int64_t]", dir: in, role: IndexedArray-index} - - {name: fromstarts, type: "Const[List[int64_t]]", dir: in, role: ListArray-starts} - - {name: fromstops, type: "Const[List[int64_t]]", dir: in, role: ListArray-stops} - - {name: ascending, type: "bool", dir: in, role: ListArray-replacement} - - {name: length, type: "int64_t", dir: in, role: default} - - {name: maxlevels, type: "int64_t", dir: in, role: default} - - name: awkward_quick_sort_int64 - args: - - {name: tmpptr, type: "List[int64_t]", dir: in, role: IndexedArray-index} - - {name: tmpbeg, type: "List[int64_t]", dir: in, role: IndexedArray-index} - - {name: tmpend, type: "List[int64_t]", dir: in, role: IndexedArray-index} - - {name: fromstarts, type: "Const[List[int64_t]]", dir: in, role: ListArray-starts} - - {name: fromstops, type: "Const[List[int64_t]]", dir: in, role: ListArray-stops} - - {name: ascending, type: "bool", dir: in, role: ListArray-replacement} - - {name: length, type: "int64_t", dir: in, role: default} - - {name: maxlevels, type: "int64_t", dir: in, role: default} - - name: awkward_quick_sort_uint8 - args: - - {name: tmpptr, type: "List[uint8_t]", dir: in, role: IndexedArray-index} - - {name: tmpbeg, type: "List[int64_t]", dir: in, role: IndexedArray-index} - - {name: tmpend, type: "List[int64_t]", dir: in, role: IndexedArray-index} - - {name: fromstarts, type: "Const[List[int64_t]]", dir: in, role: ListArray-starts} - - {name: fromstops, type: "Const[List[int64_t]]", dir: in, role: ListArray-stops} - - {name: ascending, type: "bool", dir: in, role: ListArray-replacement} - - {name: length, type: "int64_t", dir: in, role: default} - - {name: maxlevels, type: "int64_t", dir: in, role: default} - - name: awkward_quick_sort_uint16 - args: - - {name: tmpptr, type: "List[uint16_t]", dir: in, role: IndexedArray-index} - - {name: tmpbeg, type: "List[int64_t]", dir: in, role: IndexedArray-index} - - {name: tmpend, type: "List[int64_t]", dir: in, role: IndexedArray-index} - - {name: fromstarts, type: "Const[List[int64_t]]", dir: in, role: ListArray-starts} - - {name: fromstops, type: "Const[List[int64_t]]", dir: in, role: ListArray-stops} - - {name: ascending, type: "bool", dir: in, role: ListArray-replacement} - - {name: length, type: "int64_t", dir: in, role: default} - - {name: maxlevels, type: "int64_t", dir: in, role: default} - - name: awkward_quick_sort_uint32 - args: - - {name: tmpptr, type: "List[uint32_t]", dir: in, role: IndexedArray-index} - - {name: tmpbeg, type: "List[int64_t]", dir: in, role: IndexedArray-index} - - {name: tmpend, type: "List[int64_t]", dir: in, role: IndexedArray-index} - - {name: fromstarts, type: "Const[List[int64_t]]", dir: in, role: ListArray-starts} - - {name: fromstops, type: "Const[List[int64_t]]", dir: in, role: ListArray-stops} - - {name: ascending, type: "bool", dir: in, role: ListArray-replacement} - - {name: length, type: "int64_t", dir: in, role: default} - - {name: maxlevels, type: "int64_t", dir: in, role: default} - - name: awkward_quick_sort_uint64 - args: - - {name: tmpptr, type: "List[uint64_t]", dir: in, role: IndexedArray-index} - - {name: tmpbeg, type: "List[int64_t]", dir: in, role: IndexedArray-index} - - {name: tmpend, type: "List[int64_t]", dir: in, role: IndexedArray-index} - - {name: fromstarts, type: "Const[List[int64_t]]", dir: in, role: ListArray-starts} - - {name: fromstops, type: "Const[List[int64_t]]", dir: in, role: ListArray-stops} - - {name: ascending, type: "bool", dir: in, role: ListArray-replacement} - - {name: length, type: "int64_t", dir: in, role: default} - - {name: maxlevels, type: "int64_t", dir: in, role: default} - - name: awkward_quick_sort_float32 - args: - - {name: tmpptr, type: "List[float]", dir: in, role: IndexedArray-index} - - {name: tmpbeg, type: "List[int64_t]", dir: in, role: IndexedArray-index} - - {name: tmpend, type: "List[int64_t]", dir: in, role: IndexedArray-index} - - {name: fromstarts, type: "Const[List[int64_t]]", dir: in, role: ListArray-starts} - - {name: fromstops, type: "Const[List[int64_t]]", dir: in, role: ListArray-stops} - - {name: ascending, type: "bool", dir: in, role: ListArray-replacement} - - {name: length, type: "int64_t", dir: in, role: default} - - {name: maxlevels, type: "int64_t", dir: in, role: default} - - name: awkward_quick_sort_float64 - args: - - {name: tmpptr, type: "List[double]", dir: in, role: IndexedArray-index} - - {name: tmpbeg, type: "List[int64_t]", dir: in, role: IndexedArray-index} - - {name: tmpend, type: "List[int64_t]", dir: in, role: IndexedArray-index} - - {name: fromstarts, type: "Const[List[int64_t]]", dir: in, role: ListArray-starts} - - {name: fromstops, type: "Const[List[int64_t]]", dir: in, role: ListArray-stops} - - {name: ascending, type: "bool", dir: in, role: ListArray-replacement} - - {name: length, type: "int64_t", dir: in, role: default} - - {name: maxlevels, type: "int64_t", dir: in, role: default} - description: null - definition: | - Insert Python definition here - automatic-tests: false - manual-tests: [] - - name: awkward_sort specializations: - name: awkward_sort_bool From e70a9a2c18f91a9b01f1f49b5f827c52d00b7c1a Mon Sep 17 00:00:00 2001 From: Jim Pivarski Date: Thu, 12 Jan 2023 13:19:33 -0600 Subject: [PATCH 3/3] Follow PyLint's suggestion of using `in` instead of `or`. --- src/awkward/operations/ak_argsort.py | 4 ++-- src/awkward/operations/ak_sort.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/awkward/operations/ak_argsort.py b/src/awkward/operations/ak_argsort.py index 98d6c264b2..3f8b193e53 100644 --- a/src/awkward/operations/ak_argsort.py +++ b/src/awkward/operations/ak_argsort.py @@ -70,9 +70,9 @@ def _impl(array, axis, ascending, stable, highlevel, behavior): def _nep_18_impl(a, axis=-1, kind=None, order=unsupported): if kind is None: stable = False - elif kind == "stable" or kind == "mergesort": + elif kind in ("stable", "mergesort"): stable = True - elif kind == "heapsort" or kind == "quicksort": + elif kind in ("heapsort", "quicksort"): stable = False else: raise ak._errors.wrap_error( diff --git a/src/awkward/operations/ak_sort.py b/src/awkward/operations/ak_sort.py index a35b7eba1e..af9d5bd070 100644 --- a/src/awkward/operations/ak_sort.py +++ b/src/awkward/operations/ak_sort.py @@ -57,9 +57,9 @@ def _impl(array, axis, ascending, stable, highlevel, behavior): def _nep_18_impl(a, axis=-1, kind=None, order=unsupported): if kind is None: stable = False - elif kind == "stable" or kind == "mergesort": + elif kind in ("stable", "mergesort"): stable = True - elif kind == "heapsort" or kind == "quicksort": + elif kind in ("heapsort", "quicksort"): stable = False else: raise ak._errors.wrap_error(