Skip to content

Commit 995fe0a

Browse files
authored
Fix off-by-one num_segments in matrix::sort_cols_per_row (#3010)
Closes [#2049](NVIDIA/cuvs#2049) `raft::matrix::detail::sortColumnsPerRow` passed `n_rows + 1` to [cub as `num_segments`](https://github.com/NVIDIA/cccl/blob/01b00d00edff5e5ed01fa4921ed9a5ad703c3219/cub/cub/device/device_segmented_radix_sort.cuh#L217). Per the cub contract, an aliased CSR offsets array must have length `num_segments + 1`, so cub read one int past the offsets allocation. This crashes with `cudaErrorIllegalAddress` when `n_rows + 1` is a power of two ≥ 64 and `n_columns ≥ ~16384` (silent otherwise). Surfaces in `cuvs::stats::trustworthiness_score` whenever `n % batch_size ∈ {63, 127, 255, …}` (e.g. `n=76927, batch_size=512`). Fix : pass `n_rows` to cub; size the offsets array as `n_rows + 1`. Authors: - Victor Lafargue (https://github.com/viclafargue) - Corey J. Nolet (https://github.com/cjnolet) Approvers: - Divye Gala (https://github.com/divyegala) - Corey J. Nolet (https://github.com/cjnolet) - Tarang Jain (https://github.com/tarang-jain) URL: #3010
1 parent 2f724f5 commit 995fe0a

1 file changed

Lines changed: 10 additions & 5 deletions

File tree

cpp/include/raft/matrix/detail/columnWiseSort.cuh

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,12 @@ void sortColumnsPerRow(const InType* in,
221221
// device Segmented radix sort
222222
// 2^18 column cap to restrict size of workspace ~512 MB
223223
// will give better perf than below deviceWide Sort for even larger dims
224-
int numSegments = n_rows + 1;
224+
225+
// n_rows CSR-style segments aliased onto an n_rows + 1 offsets array,
226+
// with begin = &offsets[0] and end = &offsets[1]. cub's `num_segments`
227+
// is the actual segment count (n_rows).
228+
int numSegments = n_rows;
229+
int numOffsets = n_rows + 1;
225230

226231
// need auxiliary storage: cub sorting + keys (if user not passing) +
227232
// staging for values out + segment partition
@@ -248,8 +253,8 @@ void sortColumnsPerRow(const InType* in,
248253
// value in KV pair need to be passed in, out buffer is separate
249254
workspaceSize += raft::alignTo(sizeof(OutType) * (size_t)totalElements, memAlignWidth);
250255

251-
// for segment offsets
252-
workspaceSize += raft::alignTo(sizeof(int) * (size_t)numSegments, memAlignWidth);
256+
// for segment offsets (numOffsets = numSegments + 1, see above)
257+
workspaceSize += raft::alignTo(sizeof(int) * (size_t)numOffsets, memAlignWidth);
253258
} else {
254259
size_t workspaceOffset = 0;
255260

@@ -264,14 +269,14 @@ void sortColumnsPerRow(const InType* in,
264269
workspacePtr = (void*)((size_t)workspacePtr + workspaceOffset);
265270

266271
int* dSegmentOffsets = reinterpret_cast<int*>(workspacePtr);
267-
workspaceOffset = raft::alignTo(sizeof(int) * (size_t)numSegments, memAlignWidth);
272+
workspaceOffset = raft::alignTo(sizeof(int) * (size_t)numOffsets, memAlignWidth);
268273
workspacePtr = (void*)((size_t)workspacePtr + workspaceOffset);
269274

270275
// layout idx
271276
RAFT_CUDA_TRY(layoutIdx(dValuesIn, n_rows, n_columns, stream));
272277

273278
// layout segment lengths - spread out column length
274-
RAFT_CUDA_TRY(layoutSortOffset(dSegmentOffsets, n_columns, numSegments, stream));
279+
RAFT_CUDA_TRY(layoutSortOffset(dSegmentOffsets, n_columns, numOffsets, stream));
275280

276281
RAFT_CUDA_TRY(cub::DeviceSegmentedRadixSort::SortPairs(workspacePtr,
277282
workspaceSize,

0 commit comments

Comments
 (0)