feat!: support hamming distance & binary vector by BubbleCal · Pull Request #3198 · lance-format/lance

BubbleCal · 2024-12-04T09:32:21Z

No description provided.

Signed-off-by: BubbleCal <bubble-cal@outlook.com>

BubbleCal · 2024-12-04T09:36:24Z

+    pub fn nearest<T: ArrowPrimitiveType>(
+        &mut self,
+        column: &str,
+        q: &PrimitiveArray<T>,


this is a breaking change, code like this can't work since now:

.nearest(column, q.as_primitive(), k)

because the compiler can't infer the type

BubbleCal · 2024-12-04T09:38:32Z

 }
+
+#[derive(Debug, Clone, DeepSizeOf)]
+pub struct FlatBinQuantizer {


We have to add this rather than reusing FlatQuantizer even flat quantizer does nothing, because we determine the VectorStore type by Quantization

BubbleCal · 2024-12-04T09:40:42Z

    #[case(4, DistanceType::L2, 1.0)]
    #[case(4, DistanceType::Cosine, 1.0)]
    #[case(4, DistanceType::Dot, 1.0)]
+    #[case(4, DistanceType::Hamming, 0.9)]


even it's flat search, the recall for hamming can't always hit 100%, because there are many vectors that are at the same distance from the query vector

Signed-off-by: BubbleCal <bubble-cal@outlook.com>

…ector

Signed-off-by: BubbleCal <bubble-cal@outlook.com>

codecov-commenter · 2024-12-05T09:27:36Z

Codecov Report

Attention: Patch coverage is 67.40947% with 117 lines in your changes missing coverage. Please review.

Project coverage is 78.54%. Comparing base (6c7b9fd) to head (bd27747).
Report is 132 commits behind head on main.

Files with missing lines	Patch %	Lines
rust/lance-index/src/vector/flat/storage.rs	56.12%	41 Missing and 2 partials ⚠️
rust/lance-index/src/vector/flat/index.rs	53.19%	21 Missing and 1 partial ⚠️
rust/lance-linalg/src/distance/hamming.rs	36.00%	16 Missing ⚠️
rust/lance-linalg/src/kmeans.rs	82.66%	13 Missing ⚠️
rust/lance/src/index.rs	53.33%	6 Missing and 1 partial ⚠️
rust/lance-index/src/vector/quantizer.rs	0.00%	5 Missing ⚠️
rust/lance/src/dataset/scanner.rs	72.22%	4 Missing and 1 partial ⚠️
rust/lance/src/index/vector/ivf.rs	80.00%	2 Missing ⚠️
rust/lance/src/index/vector/utils.rs	0.00%	0 Missing and 2 partials ⚠️
rust/lance-index/src/vector/hnsw/builder.rs	50.00%	1 Missing ⚠️
... and 1 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3198      +/-   ##
==========================================
- Coverage   78.55%   78.54%   -0.01%     
==========================================
  Files         244      244              
  Lines       83213    84298    +1085     
  Branches    83213    84298    +1085     
==========================================
+ Hits        65370    66216     +846     
- Misses      15056    15286     +230     
- Partials     2787     2796       +9

Flag	Coverage Δ
unittests	`78.54% <67.40%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: BubbleCal <bubble-cal@outlook.com>

…ector Signed-off-by: BubbleCal <bubble-cal@outlook.com>

westonpace

Might still be good for @chebbyChefNEQ or @eddyxu to look at the indexing code but the rest looks ok to me.

Looks like we needed to duplicate a few structs to have a "Bin" version and a "Float" version?

westonpace · 2024-12-05T14:50:25Z

+    // deprecated, use `try_from_batch` instead
+    pub fn new(vectors: FixedSizeListArray, distance_type: DistanceType) -> Self {
+        let row_ids = Arc::new(UInt64Array::from_iter_values(0..vectors.len() as u64));
+        let vectors = Arc::new(vectors);
+
+        let batch = RecordBatch::try_from_iter_with_nullable(vec![
+            (ROW_ID, row_ids.clone() as ArrayRef, true),
+            (FLAT_COLUMN, vectors.clone() as ArrayRef, true),
+        ])
+        .unwrap();
+
+        Self {
+            batch,
+            distance_type,
+            row_ids,
+            vectors,
+        }
+    }


Can we avoid this deprecated method in Bin path?

right, deleted

westonpace · 2024-12-05T14:56:14Z

+                } else if dt.data_type().is_floating() {
+                    coerce_float_vector(
+                        q.as_any().downcast_ref::<Float32Array>().unwrap(),
+                        FloatType::try_from(dt.data_type())?,
+                    )?


I think the error message isn't quite right here. If dt is not a floating point type and q is a vector, but does not exactly match dt (e.g. UInt16 or something) then the error the user will get is Column {} is not a vector column. However, it would be nicer to give them an error like Column {} has type UInt8 and the query vector is UInt16.

good point, fixed!

westonpace · 2024-12-05T14:58:30Z

-            .try_collect::<Vec<_>>()
-            .await?;
-        concat_batches(&Arc::new(ArrowSchema::from(&projection)), &batches)?
+        Box::pin(scanner.try_into_batch()).await?


I wonder if it would be more thorough for try_into_batch to return a boxed future instead of being async?

BubbleCal · 2024-12-06T03:52:51Z

Might still be good for @chebbyChefNEQ or @eddyxu to look at the indexing code but the rest looks ok to me.

Looks like we needed to duplicate a few structs to have a "Bin" version and a "Float" version?

Quantization determines the VectorStore, VectorStore determines the DistCalculator. For best performance we are storing the vectors ref / query as reference to slice of primitive type (f32/u8) in DistCalculator, so yes now we have to dup the structs to make the type system happy.

it's avoidable, by making DistCalculator store ArrayRef, but will make search slower

Signed-off-by: BubbleCal <bubble-cal@outlook.com>

eddyxu · 2024-12-06T20:53:24Z

 resolver = "2"

 [workspace.package]
-version = "0.20.1"


This is not relevant?

this PR introduces a breaking change so bump the version

eddyxu · 2024-12-06T20:55:32Z

    let data = generate_random_array_with_seed::<Float32Type>(TOTAL * DIMENSION, SEED);
    let fsl = FixedSizeListArray::try_new_from_values(data, DIMENSION as i32).unwrap();
-    let vectors = Arc::new(FlatStorage::new(fsl.clone(), DistanceType::L2));
+    let vectors = Arc::new(FlatFloatStorage::new(fsl.clone(), DistanceType::L2));


Ah, do we need FlatStorage to be typed? This is sad

yeah... unfortunately now we have to do this.
there are 2 ways to avoid this in my mind but they would introduce some new issues as well:

storing ArrayRef in dist calculator then cast it to &PrimitiveArray<T> according distance type / column type. This is what we did before, it's slow

just storing everything in bytes ref &[u8] then do unsafe cast

feat: support hamming distance & binary vector

cd4651e

Signed-off-by: BubbleCal <bubble-cal@outlook.com>

github-actions Bot added the enhancement New feature or request label Dec 4, 2024

BubbleCal commented Dec 4, 2024

View reviewed changes

BubbleCal changed the title ~~feat: support hamming distance & binary vector~~ feat!: support hamming distance & binary vector Dec 4, 2024

github-actions Bot added the breaking-change label Dec 4, 2024

make clippy happy

a2af388

Signed-off-by: BubbleCal <bubble-cal@outlook.com>

BubbleCal mentioned this pull request Dec 4, 2024

feat: support binary vector and hamming distance #3199

Closed

2 tasks

fix

aa206a2

Signed-off-by: BubbleCal <bubble-cal@outlook.com>

github-actions Bot added the python label Dec 5, 2024

Merge branch 'main' of https://github.com/lancedb/lance into binary-v…

f3211b0

…ector

BubbleCal mentioned this pull request Dec 5, 2024

feat: support build IVF_FLAT with binary vectors #2434

Closed

BubbleCal added 5 commits December 5, 2024 14:31

fix

4f279ea

Signed-off-by: BubbleCal <bubble-cal@outlook.com>

update Cargo.lock

085db49

Signed-off-by: BubbleCal <bubble-cal@outlook.com>

fix stack overflow

1a2893b

Signed-off-by: BubbleCal <bubble-cal@outlook.com>

fix stack overflow

844fa3c

Signed-off-by: BubbleCal <bubble-cal@outlook.com>

fix

b52af6d

Signed-off-by: BubbleCal <bubble-cal@outlook.com>

BubbleCal mentioned this pull request Dec 5, 2024

feat: support binary & float vectors for IVF_FLAT index #2461

Closed

fix stackoverflow

d951bed

Signed-off-by: BubbleCal <bubble-cal@outlook.com>

BubbleCal marked this pull request as ready for review December 5, 2024 12:18

BubbleCal requested review from eddyxu, westonpace and wjones127 December 5, 2024 12:21

Merge branch 'main' of https://github.com/lancedb/lance into binary-v…

6e428fc

…ector Signed-off-by: BubbleCal <bubble-cal@outlook.com>

westonpace approved these changes Dec 5, 2024

View reviewed changes

BubbleCal added 2 commits December 6, 2024 12:06

move large future to heap

1c9561d

Signed-off-by: BubbleCal <bubble-cal@outlook.com>

fix error message

4ace2d0

Signed-off-by: BubbleCal <bubble-cal@outlook.com>

BubbleCal added 2 commits December 6, 2024 12:22

bump version

07e2197

Signed-off-by: BubbleCal <bubble-cal@outlook.com>

fix version

bd27747

Signed-off-by: BubbleCal <bubble-cal@outlook.com>

eddyxu reviewed Dec 6, 2024

View reviewed changes

BubbleCal requested a review from eddyxu December 7, 2024 06:41

BubbleCal merged commit 4444c60 into lance-format:main Dec 7, 2024

Conversation

BubbleCal commented Dec 4, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Dec 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

westonpace left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BubbleCal commented Dec 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BubbleCal Dec 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Dec 5, 2024 •

edited

Loading

BubbleCal commented Dec 6, 2024 •

edited

Loading

BubbleCal Dec 7, 2024 •

edited

Loading