Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: impl the sub index trait for HNSW and clean code #2462

Merged
merged 13 commits into from
Jun 13, 2024

Conversation

BubbleCal
Copy link
Contributor

@BubbleCal BubbleCal commented Jun 11, 2024

  • impl IvfSubIndex for HNSW
  • remove IO related methods from HNSW
  • remove unused Result return type for some HNSW methods

Signed-off-by: BubbleCal <bubble-cal@outlook.com>
Signed-off-by: BubbleCal <bubble-cal@outlook.com>
@codecov-commenter
Copy link

codecov-commenter commented Jun 12, 2024

Codecov Report

Attention: Patch coverage is 81.52174% with 68 lines in your changes missing coverage. Please review.

Project coverage is 80.01%. Comparing base (848a5db) to head (1aa91de).
Report is 2 commits behind head on main.

Files Patch % Lines
rust/lance-index/src/vector/hnsw/builder.rs 85.33% 24 Missing and 15 partials ⚠️
rust/lance/src/index/vector.rs 0.00% 12 Missing ⚠️
rust/lance-index/src/vector/hnsw/index.rs 66.66% 3 Missing and 4 partials ⚠️
rust/lance/src/index/vector/builder.rs 90.90% 0 Missing and 3 partials ⚠️
rust/lance/src/index/vector/ivf.rs 60.00% 0 Missing and 2 partials ⚠️
rust/lance/src/index/vector/ivf/io.rs 50.00% 0 Missing and 2 partials ⚠️
rust/lance-core/src/datatypes/field.rs 0.00% 1 Missing ⚠️
rust/lance-index/src/vector/flat/index.rs 88.88% 1 Missing ⚠️
rust/lance-index/src/vector/hnsw.rs 90.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2462      +/-   ##
==========================================
+ Coverage   79.94%   80.01%   +0.07%     
==========================================
  Files         207      207              
  Lines       58657    58735      +78     
  Branches    58657    58735      +78     
==========================================
+ Hits        46893    46997     +104     
+ Misses       9005     8982      -23     
+ Partials     2759     2756       -3     
Flag Coverage Δ
unittests 80.01% <81.52%> (+0.07%) ⬆️

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>
Signed-off-by: BubbleCal <bubble-cal@outlook.com>
Signed-off-by: BubbleCal <bubble-cal@outlook.com>
Signed-off-by: BubbleCal <bubble-cal@outlook.com>
Signed-off-by: BubbleCal <bubble-cal@outlook.com>
Signed-off-by: BubbleCal <bubble-cal@outlook.com>
Signed-off-by: BubbleCal <bubble-cal@outlook.com>
Signed-off-by: BubbleCal <bubble-cal@outlook.com>
Signed-off-by: BubbleCal <bubble-cal@outlook.com>
Signed-off-by: BubbleCal <bubble-cal@outlook.com>
use lance_table::io::manifest::ManifestDescribing;

use lance_linalg::distance::DistanceType;
use rayon::prelude::*;
Copy link
Contributor

Choose a reason for hiding this comment

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

What is rayon used for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's used to do parallel insert for building HNSW

Comment on lines 562 to 568
let ids = batch
.column_by_name(VECTOR_ID_COL)
.ok_or(Error::Index {
message: format!("{} column not found in HNSW file", VECTOR_ID_COL),
location: location!(),
})?
.as_primitive::<UInt32Type>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be simpler to just do:

let ids = batch[VECTOR_ID_COL].as_primitive::<UInt32Type>();

It's a panic instead of an error but users won't be able to understand or handle the error anyways.

Signed-off-by: BubbleCal <bubble-cal@outlook.com>
@BubbleCal BubbleCal merged commit d830c38 into lancedb:main Jun 13, 2024
18 of 19 checks passed
renovate bot referenced this pull request in spiraldb/vortex Jun 13, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [lance](https://togithub.com/lancedb/lance) | dependencies | patch |
`0.12.1` -> `0.12.2` |

---

### Release Notes

<details>
<summary>lancedb/lance (lance)</summary>

### [`v0.12.2`](https://togithub.com/lancedb/lance/releases/tag/v0.12.2)

[Compare
Source](https://togithub.com/lancedb/lance/compare/v0.12.1...v0.12.2)

<!-- Release notes generated using configuration in .github/release.yml
at v0.12.2 -->

#### What's Changed

##### New Features 🎉

- feat: support IVF_FLAT index by
[@&#8203;BubbleCal](https://togithub.com/BubbleCal) in
[https://github.com/lancedb/lance/pull/2418](https://togithub.com/lancedb/lance/pull/2418)
- feat: add support for arbitrary global buffers to the v2 reader/writer
by [@&#8203;westonpace](https://togithub.com/westonpace) in
[https://github.com/lancedb/lance/pull/2445](https://togithub.com/lancedb/lance/pull/2445)
- feat: add pushdown to the read path by
[@&#8203;westonpace](https://togithub.com/westonpace) in
[https://github.com/lancedb/lance/pull/2444](https://togithub.com/lancedb/lance/pull/2444)
- feat: scan with stable row id by
[@&#8203;wjones127](https://togithub.com/wjones127) in
[https://github.com/lancedb/lance/pull/2441](https://togithub.com/lancedb/lance/pull/2441)
- feat(java): add lance spark connector for read by
[@&#8203;LuQQiu](https://togithub.com/LuQQiu) in
[https://github.com/lancedb/lance/pull/2429](https://togithub.com/lancedb/lance/pull/2429)
- feat: pickle the manifest when pickling a dataset by
[@&#8203;westonpace](https://togithub.com/westonpace) in
[https://github.com/lancedb/lance/pull/2459](https://togithub.com/lancedb/lance/pull/2459)
- feat: store IVF in global buffer by
[@&#8203;BubbleCal](https://togithub.com/BubbleCal) in
[https://github.com/lancedb/lance/pull/2449](https://togithub.com/lancedb/lance/pull/2449)

##### Bug Fixes 🐛

- fix: lance.torch.LanceDataset spelt readahead wrong by
[@&#8203;wjones127](https://togithub.com/wjones127) in
[https://github.com/lancedb/lance/pull/2453](https://togithub.com/lancedb/lance/pull/2453)
- fix: respect storage options on `create_index` by
[@&#8203;joshua-auchincloss](https://togithub.com/joshua-auchincloss) in
[https://github.com/lancedb/lance/pull/2460](https://togithub.com/lancedb/lance/pull/2460)
- fix: incorrect logic when decoding manifest from last block by
[@&#8203;westonpace](https://togithub.com/westonpace) in
[https://github.com/lancedb/lance/pull/2464](https://togithub.com/lancedb/lance/pull/2464)

##### Documentation 📚

- docs: add spark-lance-connector readme by
[@&#8203;LuQQiu](https://togithub.com/LuQQiu) in
[https://github.com/lancedb/lance/pull/2456](https://togithub.com/lancedb/lance/pull/2456)

##### Performance Improvements 🚀

- perf: speed up hnsw build by
[@&#8203;eddyxu](https://togithub.com/eddyxu) in
[https://github.com/lancedb/lance/pull/2448](https://togithub.com/lancedb/lance/pull/2448)
- perf: better L2 code generation by
[@&#8203;eddyxu](https://togithub.com/eddyxu) in
[https://github.com/lancedb/lance/pull/2450](https://togithub.com/lancedb/lance/pull/2450)

##### Other Changes

- refactor: pull row_id and deletion vector out of v1 file reader by
[@&#8203;wjones127](https://togithub.com/wjones127) in
[https://github.com/lancedb/lance/pull/2436](https://togithub.com/lancedb/lance/pull/2436)
- refactor: move Prefilter to a trait and move VectorIndex to
lance-vector by [@&#8203;eddyxu](https://togithub.com/eddyxu) in
[https://github.com/lancedb/lance/pull/2442](https://togithub.com/lancedb/lance/pull/2442)
- refactor: move hnsw from lance crate to lance-index crate by
[@&#8203;eddyxu](https://togithub.com/eddyxu) in
[https://github.com/lancedb/lance/pull/2446](https://togithub.com/lancedb/lance/pull/2446)
- refactor: impl the sub index trait for HNSW and clean code by
[@&#8203;BubbleCal](https://togithub.com/BubbleCal) in
[https://github.com/lancedb/lance/pull/2462](https://togithub.com/lancedb/lance/pull/2462)

#### New Contributors

- [@&#8203;joshua-auchincloss](https://togithub.com/joshua-auchincloss)
made their first contribution in
[https://github.com/lancedb/lance/pull/2460](https://togithub.com/lancedb/lance/pull/2460)

**Full Changelog**:
lancedb/lance@v0.12.1...v0.12.2

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/spiraldb/vortex).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zOTMuMCIsInVwZGF0ZWRJblZlciI6IjM3LjM5My4wIiwidGFyZ2V0QnJhbmNoIjoiZGV2ZWxvcCIsImxhYmVscyI6W119-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
AdamGS referenced this pull request in AdamGS/vortex Jun 14, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [lance](https://togithub.com/lancedb/lance) | dependencies | patch |
`0.12.1` -> `0.12.2` |

---

### Release Notes

<details>
<summary>lancedb/lance (lance)</summary>

### [`v0.12.2`](https://togithub.com/lancedb/lance/releases/tag/v0.12.2)

[Compare
Source](https://togithub.com/lancedb/lance/compare/v0.12.1...v0.12.2)

<!-- Release notes generated using configuration in .github/release.yml
at v0.12.2 -->

#### What's Changed

##### New Features 🎉

- feat: support IVF_FLAT index by
[@&#8203;BubbleCal](https://togithub.com/BubbleCal) in
[https://github.com/lancedb/lance/pull/2418](https://togithub.com/lancedb/lance/pull/2418)
- feat: add support for arbitrary global buffers to the v2 reader/writer
by [@&#8203;westonpace](https://togithub.com/westonpace) in
[https://github.com/lancedb/lance/pull/2445](https://togithub.com/lancedb/lance/pull/2445)
- feat: add pushdown to the read path by
[@&#8203;westonpace](https://togithub.com/westonpace) in
[https://github.com/lancedb/lance/pull/2444](https://togithub.com/lancedb/lance/pull/2444)
- feat: scan with stable row id by
[@&#8203;wjones127](https://togithub.com/wjones127) in
[https://github.com/lancedb/lance/pull/2441](https://togithub.com/lancedb/lance/pull/2441)
- feat(java): add lance spark connector for read by
[@&#8203;LuQQiu](https://togithub.com/LuQQiu) in
[https://github.com/lancedb/lance/pull/2429](https://togithub.com/lancedb/lance/pull/2429)
- feat: pickle the manifest when pickling a dataset by
[@&#8203;westonpace](https://togithub.com/westonpace) in
[https://github.com/lancedb/lance/pull/2459](https://togithub.com/lancedb/lance/pull/2459)
- feat: store IVF in global buffer by
[@&#8203;BubbleCal](https://togithub.com/BubbleCal) in
[https://github.com/lancedb/lance/pull/2449](https://togithub.com/lancedb/lance/pull/2449)

##### Bug Fixes 🐛

- fix: lance.torch.LanceDataset spelt readahead wrong by
[@&#8203;wjones127](https://togithub.com/wjones127) in
[https://github.com/lancedb/lance/pull/2453](https://togithub.com/lancedb/lance/pull/2453)
- fix: respect storage options on `create_index` by
[@&#8203;joshua-auchincloss](https://togithub.com/joshua-auchincloss) in
[https://github.com/lancedb/lance/pull/2460](https://togithub.com/lancedb/lance/pull/2460)
- fix: incorrect logic when decoding manifest from last block by
[@&#8203;westonpace](https://togithub.com/westonpace) in
[https://github.com/lancedb/lance/pull/2464](https://togithub.com/lancedb/lance/pull/2464)

##### Documentation 📚

- docs: add spark-lance-connector readme by
[@&#8203;LuQQiu](https://togithub.com/LuQQiu) in
[https://github.com/lancedb/lance/pull/2456](https://togithub.com/lancedb/lance/pull/2456)

##### Performance Improvements 🚀

- perf: speed up hnsw build by
[@&#8203;eddyxu](https://togithub.com/eddyxu) in
[https://github.com/lancedb/lance/pull/2448](https://togithub.com/lancedb/lance/pull/2448)
- perf: better L2 code generation by
[@&#8203;eddyxu](https://togithub.com/eddyxu) in
[https://github.com/lancedb/lance/pull/2450](https://togithub.com/lancedb/lance/pull/2450)

##### Other Changes

- refactor: pull row_id and deletion vector out of v1 file reader by
[@&#8203;wjones127](https://togithub.com/wjones127) in
[https://github.com/lancedb/lance/pull/2436](https://togithub.com/lancedb/lance/pull/2436)
- refactor: move Prefilter to a trait and move VectorIndex to
lance-vector by [@&#8203;eddyxu](https://togithub.com/eddyxu) in
[https://github.com/lancedb/lance/pull/2442](https://togithub.com/lancedb/lance/pull/2442)
- refactor: move hnsw from lance crate to lance-index crate by
[@&#8203;eddyxu](https://togithub.com/eddyxu) in
[https://github.com/lancedb/lance/pull/2446](https://togithub.com/lancedb/lance/pull/2446)
- refactor: impl the sub index trait for HNSW and clean code by
[@&#8203;BubbleCal](https://togithub.com/BubbleCal) in
[https://github.com/lancedb/lance/pull/2462](https://togithub.com/lancedb/lance/pull/2462)

#### New Contributors

- [@&#8203;joshua-auchincloss](https://togithub.com/joshua-auchincloss)
made their first contribution in
[https://github.com/lancedb/lance/pull/2460](https://togithub.com/lancedb/lance/pull/2460)

**Full Changelog**:
lancedb/lance@v0.12.1...v0.12.2

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/spiraldb/vortex).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zOTMuMCIsInVwZGF0ZWRJblZlciI6IjM3LjM5My4wIiwidGFyZ2V0QnJhbmNoIjoiZGV2ZWxvcCIsImxhYmVscyI6W119-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants