Skip to content

fix(index): store the converted metric in IvfTransformer::new_flat - #9399

Open
LuciferYang wants to merge 3 commits into
lance-format:mainfrom
LuciferYang:fix/ivf-rq-newflat-distance-type
Open

LuciferYang wants to merge 3 commits into
lance-format:mainfrom
LuciferYang:fix/ivf-rq-newflat-distance-type

Conversation

@LuciferYang

Copy link
Copy Markdown
Contributor

Closes #9398

new_flat converts cosine to L2 for its transform chain (normalize, then assign with L2) and then handed the original metric to IvfTransformer::new, unlike with_pq and the constructor above it. The stored metric is what the transformer's own find_partitions, compute_partitions, and compute_residual use, so a cosine flat transformer measured in cosine while its chain had assigned in L2. It now stores dt, so all four agree.

Partition choice is unchanged: the vectors and centroids are normalized by then, and cosine and L2 rank unit vectors the same way. What changes is the scale of the distances those three methods return, which now matches the __ivf_centroid_dist values the chain produces.

How was this patch tested?

test_new_flat_keeps_the_converted_metric builds a cosine and an L2 flat transformer over the same unit centroids and asserts find_partitions returns the same partitions and the same distances for both. With the original metric stored, the cosine one returns cosine distances and the test fails. The file had no test module before, so this adds one.

new_flat computed the cosine-converted L2 metric for its
PartitionTransformer but stored the original distance type on
IvfTransformer, so find_partitions/compute_partitions assigned with
Cosine over data partitioned under L2-on-normalized. Store the
converted metric like every sibling constructor.

Assisted-by: GLM-5.3
@github-actions github-actions Bot added bug Something isn't working A-index Vector index, linalg, tokenizer and removed bug Something isn't working labels Sep 18, 2026

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gate recommendation: approve.

Storing the converted L2 metric matches the transformer’s existing normalize-then-L2 contract and prevents the Cosine variant from reaching unsupported partition-distance code. The added regression covers this invariant; I found no blocking or material residual risk.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-index Vector index, linalg, tokenizer K-approved Latest Gatekeeper recommendation permits acceptance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: IvfTransformer::new_flat stores the unconverted metric for cosine

1 participant