NVIDIA / NVIDIA/cudf

[BUG] percentile_approx returns non-empty null list rows for mixed empty and non-empty TDigests

Open
#24,056 0 comments 0 reactions 1 assignee Claimed by @davidwendt View on GitHub
? - Needs Triage bug libcudf Spark
Dominant language
C++
Stars
9.8k
Forks
1.1k
Avg merge
3d 6m
Merged PRs (30d)
278

Description

**Describe the bug**

`cudf::percentile_approx` can return a LIST column with non-empty null rows when its
TDigest input mixes empty and non-empty digests. The empty digest rows are marked null,
but their list offsets still reserve `percentiles.size()` child elements. This violates
the nested-column invariant expected by downstream consumers; the Java API reports:

```text
Column has non-empty nulls
```

This is related to #10856 / #11498, which sanitize the all-empty-input case. The mixed
case still takes the regular output path and remains unsanitized.

The problem was originally observed by cuDF for Apache Spark in
NVIDIA/cudf-spark#14634.

**Steps/Code to reproduce bug**

The following libcudf gtest setup reproduces the mixed case on the validated `main`
base (`2cf22a1fb5b864bd53128034d34217943bea0047`):

```cpp
auto const values =
cudf::test::fixed_width_column_wrapper{{1, 0, 3}, {true, false, true}};
auto const keys = cudf::test::fixed_width_column_wrapper{0, 1, 2};
auto const percentiles = cudf::test::fixed_width_column_wrapper{0.0, 0.5, 1.0};

cudf::groupby::groupby gb(
cudf::table_view{{keys}}, cudf::null_policy::EXCLUDE, cudf::sorted::YES);
std::vector requests;
std::vector> aggregations;
aggregations.push_back(cudf::make_tdigest_aggregation(1000));
requests.push_back({values, std::move(aggregations)});
auto const tdigest_column = gb.aggregate(requests);

cudf::tdigest::tdigest_column_view tdv(*tdigest_column.second[0].results[0]);
auto const result = cudf::percentile_approx(tdv, percentiles);
```

An equivalent pre-fix native regression run failed with:

```text
C++ exception with description "lhs column has non-empty nulls" thrown in the test body.
[ PASSED ] 0 tests.
[ FAILED ] 1 test
```

The regular path currently creates fixed-width offsets for every digest, nullifies empty
digest rows, and leaves the child slots for those null rows in place. The percentile
kernel does not populate those slots.

**Expected behavior**

For the input above, the result should have:

```text
offsets: [0, 3, 3, 6]
child: [1, 1, 1, 3, 3, 3]
validity: [true, false, true]
```

In other words, an empty digest should produce a null list row with zero child elements.

A local candidate fix (`0ee212f124a9970cffb9a99c13766f58e7bed429`) gives empty digests zero-length list ranges and writes non-empty
digests directly into a compact child column. The no-empty kernel path remains dense and
does not load output offsets. Validation includes a nullable percentile and mixed-input
validity-mask boundaries at 31, 32, 33, 65, and 257 rows:

```text
[ PASSED ] 2 tests. # focused mixed empty/non-empty regressions
[==========] 388 tests from 67 test suites ran.
[ PASSED ] 388 tests.
```

**Environment overview (please complete the following information)**

- Environment location: bare metal
- Method of cuDF install: built from source
- Build host: NVIDIA A100, CUDA 12.9.1
- Runtime GPU: NVIDIA RTX 5880 Ada Generation
- CUDA architectures: `80-real;89-real`

**Environment details**

The failure and passing validation above are native libcudf `QUANTILES_TEST` runs from
the stated source commit. A full Spark/JNI validation requires rebuilding the combined
spark-rapids-jni native library; swapping only standalone `libcudf.so` into an existing
JNI artifact is not ABI-complete.

**Additional context**

The proposed change is localized to `cpp/src/quantiles/tdigest/tdigest.cu`. For mixed
input, build list offsets with row size zero for empty digests, allocate only the compact
child size, and write valid digest results through those offsets. This avoids an
additional full-column gather and its temporary output allocation. The no-empty fast
path is unchanged. The percentile kernel uses a bounded launch and a grid-stride loop so
a sparse compact output cannot request an invalid CUDA grid from the larger dense pair
count.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.