Skip to content

vector_of_kll: convert array inputs to the expected dtype - #73

Merged
leerho merged 1 commit into
mainfrom
vector-kll-dtypes
Oct 11, 2026
Merged

leerho merged 1 commit into
mainfrom
vector-kll-dtypes

Conversation

@leerho

@leerho leerho commented Oct 10, 2026

Copy link
Copy Markdown
Member

Fixes #63.

Problem

The vector_of_kll_*_sketches query methods accept a scalar, a list, or a numpy array. The array alternative was an untyped nb::ndarray<>, and input_to_vec() wrapped its buffer as the expected element type without checking the dtype, so a mismatched array was silently reinterpreted. Reproduced on main:

Call on vector_of_kll_floats_sketches main this PR
get_quantiles(np.float32 ranks) (the issue) [-98.9, -99.5] [-99.2, -10.0], same as float64
get_ranks(np.float64 values) [1.0, 1.0] same as a list
isk=np.array([0, 1]) (int64, numpy's default) sketch 0 twice sketches 0 and 1

update() was not affected: it already takes a typed nb::ndarray<T>, which nanobind converts.

Fix

ArrInputType now uses nb::ndarray<V, nb::ndim<1>> for its array alternative, so nanobind converts the input to the expected dtype before the method sees it. This covers get_quantiles, get_ranks, get_pmf, get_cdf, and the isk argument of every method that takes it (collapse, serialize, ...).

One behavior change: a 2-D array passed where a 1-D input is expected now raises TypeError. Before, its buffer was reinterpreted as 1-D.

Testing

New test_kll_input_dtypes checks that float32 and float64 arrays give the same results as lists for get_quantiles, get_ranks, get_pmf and get_cdf, and that int32 and int64 index arrays select the same sketches as a list. It fails on main and passes with this change; all 39 tests pass (macOS, Python 3.12). The ranks in the test are exactly representable in float32 (0.0625, 0.9375); a value like np.float32(0.99) is genuinely a different rank from 0.99 and can select a different item.

🤖 Generated with Claude Code

The query methods took array arguments as an untyped nb::ndarray<> and
input_to_vec() wrapped the buffer as the expected element type without
checking its dtype. A float32 ranks array was read as float64, a float64
values array was read as float32 by the floats sketches, and an int64
index array (numpy's default int) was read as int32, all silently.

Type the array alternative of ArrInputType as nb::ndarray<V, nb::ndim<1>>
so nanobind converts the input to the expected dtype. This fixes
get_quantiles, get_ranks, get_pmf, get_cdf, and the isk index argument
of every method that takes one.

Fixes #63.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@leerho
leerho requested review from proost and tisonkun October 10, 2026 21:57
@leerho
leerho merged commit bd0231d into main Oct 11, 2026
6 checks passed
@leerho
leerho deleted the vector-kll-dtypes branch October 11, 2026 02:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vector_of_kll_floats_sketches.get_quantiles() returns wrong values with float32

2 participants