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

feat: make scalar index training configurable #2686

Merged
merged 5 commits into from
Aug 5, 2024

Conversation

dsgibbons
Copy link
Contributor

Closes #2661.

This PR adds a scan_unordered_chunks method to the TrainingSource trait and provides basic implementations where required. train_bitmap_index has been updated to take advantage of this new method. Please advise if any other indexing algorithms do not require sorted columns and I'll update accordingly.

@github-actions github-actions bot added the enhancement New feature or request label Aug 4, 2024
@dsgibbons
Copy link
Contributor Author

@westonpace looks like UnnestTrainingSource also needs scan_unordered_chunks to be implemented as train_label_list_index calls train_bitmap_index.

@codecov-commenter
Copy link

codecov-commenter commented Aug 4, 2024

Codecov Report

Attention: Patch coverage is 76.19048% with 10 lines in your changes missing coverage. Please review.

Project coverage is 79.45%. Comparing base (712405e) to head (5105a01).
Report is 5 commits behind head on main.

Files Patch % Lines
rust/lance-index/src/scalar/btree.rs 0.00% 4 Missing ⚠️
rust/lance/src/index/scalar.rs 81.25% 0 Missing and 3 partials ⚠️
rust/lance-index/src/scalar/label_list.rs 88.88% 1 Missing and 1 partial ⚠️
rust/lance-index/src/scalar/bitmap.rs 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2686      +/-   ##
==========================================
- Coverage   79.61%   79.45%   -0.17%     
==========================================
  Files         226      227       +1     
  Lines       66516    66920     +404     
  Branches    66516    66920     +404     
==========================================
+ Hits        52955    53168     +213     
- Misses      10465    10626     +161     
- Partials     3096     3126      +30     
Flag Coverage Δ
unittests 79.45% <76.19%> (-0.17%) ⬇️

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.

Copy link
Contributor

@BubbleCal BubbleCal left a comment

Choose a reason for hiding this comment

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

Great work, thanks!

Copy link
Contributor

@westonpace westonpace left a comment

Choose a reason for hiding this comment

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

Agreed. This looks great, just a few minor naming suggestions.

rust/lance-index/src/scalar/btree.rs Outdated Show resolved Hide resolved
rust/lance-index/src/scalar/label_list.rs Outdated Show resolved Hide resolved
@westonpace
Copy link
Contributor

FYI: I filed the original issue because I was doing some testing on a dataset with 1 billion rows and after 90 minutes it still had not created a bitmap index on a boolean column. I just tested with this PR and was able to create a bitmap index in 90 seconds 🎉

@dsgibbons
Copy link
Contributor Author

Agreed. This looks great, just a few minor naming suggestions.

Done!

@westonpace westonpace merged commit 2480164 into lancedb:main Aug 5, 2024
22 checks passed
@dsgibbons dsgibbons deleted the perf/2661 branch August 15, 2024 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Speed up bitmap index creation by skipping sort
4 participants