Skip to content

Commit e67ae2f

Browse files
authored
FIX: Use intro sort for simultaneous_sort (2 way partitioning) (scikit-learn#33252)
1 parent a75111f commit e67ae2f

8 files changed

Lines changed: 295 additions & 157 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
- Fixed :func:`metrics.pairwise_distances_argmin` and
2+
:func:`metrics.pairwise_distances_argmin_min` to avoid a quadratic-time path
3+
when many distances are identical, which could lead to severe slowdowns or
4+
even a stack overflow (segmentation fault) on large inputs.
5+
By :user:`Arthur Lacote <cakedev0>`.
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
- Fixed a quadratic-time path in the internal ``simultaneous_sort`` used by
2+
:class:`neighbors.BallTree` and :class:`neighbors.KDTree` queries when many
3+
distances are identical, which could lead to severe slowdowns or even a stack
4+
overflow (segmentation fault) on large inputs. Neighbor searches with tied
5+
distances no longer degrade badly in runtime.
6+
By :user:`Arthur Lacote <cakedev0>`.

sklearn/neighbors/_binary_tree.pxi.tp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -583,9 +583,9 @@ cdef class NeighborsHeap{{name_suffix}}:
583583
cdef intp_t row
584584
for row in range(self.distances.shape[0]):
585585
_simultaneous_sort(
586-
dist=&self.distances[row, 0],
587-
idx=&self.indices[row, 0],
588-
size=self.distances.shape[1],
586+
values=&self.distances[row, 0],
587+
indices=&self.indices[row, 0],
588+
n=self.distances.shape[1],
589589
)
590590
return 0
591591

sklearn/tree/_criterion.pyx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ from scipy.special.cython_special cimport xlogy
1313

1414
from sklearn.tree._utils cimport log
1515
from sklearn.tree._utils cimport WeightedFenwickTree
16-
from sklearn.utils._sorting cimport sort
16+
from sklearn.utils._sorting cimport simultaneous_sort
1717

1818
# EPSILON is used in the Poisson criterion
1919
cdef float64_t EPSILON = 10 * np.finfo('double').eps
@@ -1138,7 +1138,7 @@ cdef inline void compute_ranks(
11381138
cdef intp_t i
11391139
for i in range(n):
11401140
sorted_indices[i] = i
1141-
sort(sorted_y, sorted_indices, n)
1141+
simultaneous_sort(sorted_y, sorted_indices, n, use_three_way_partition=True)
11421142
for i in range(n):
11431143
ranks[sorted_indices[i]] = i
11441144

sklearn/tree/_partitioner.pyx

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ cnp.import_array()
2121
from scipy.sparse import issparse
2222

2323
from sklearn.tree._splitter cimport SplitRecord
24-
from sklearn.utils._sorting cimport sort
24+
from sklearn.utils._sorting cimport simultaneous_sort
2525

2626
# Constant to switch between algorithm non zero value extract algorithm
2727
# in SparsePartitioner
@@ -100,7 +100,12 @@ cdef class DensePartitioner:
100100
for i in range(self.start, self.end):
101101
feature_values[i] = X[samples[i], current_feature]
102102

103-
sort(&feature_values[self.start], &samples[self.start], self.end - self.start - n_missing)
103+
simultaneous_sort(
104+
&feature_values[self.start],
105+
&samples[self.start],
106+
self.end - self.start - n_missing,
107+
use_three_way_partition=True,
108+
)
104109
self.n_missing = n_missing
105110

106111
cdef void shift_missing_to_the_left(self) noexcept nogil:
@@ -331,12 +336,18 @@ cdef class SparsePartitioner:
331336

332337
self.extract_nnz(current_feature)
333338
# Sort the positive and negative parts of `feature_values`
334-
sort(&feature_values[self.start], &samples[self.start], self.end_negative - self.start)
339+
simultaneous_sort(
340+
&feature_values[self.start],
341+
&samples[self.start],
342+
self.end_negative - self.start,
343+
use_three_way_partition=True,
344+
)
335345
if self.start_positive < self.end:
336-
sort(
346+
simultaneous_sort(
337347
&feature_values[self.start_positive],
338348
&samples[self.start_positive],
339-
self.end - self.start_positive
349+
self.end - self.start_positive,
350+
use_three_way_partition=True,
340351
)
341352

342353
# Update index_to_samples to take into account the sort

sklearn/utils/_sorting.pxd

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,9 @@ from sklearn.utils._typedefs cimport intp_t
22

33
from cython cimport floating
44

5-
cdef int simultaneous_sort(
6-
floating *dist,
7-
intp_t *idx,
8-
intp_t size,
9-
) noexcept nogil
10-
11-
cdef void sort(
12-
floating* feature_values,
13-
intp_t* samples,
5+
cdef void simultaneous_sort(
6+
floating* values,
7+
intp_t* indices,
148
intp_t n,
9+
bint use_three_way_partition=*,
1510
) noexcept nogil

0 commit comments

Comments
 (0)