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.

Sign up for GitHub

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

Jump to bottom

Allow host dataset for IVF-PQ #1114

Merged
merged 4 commits into from
Jan 10, 2023
Merged

Allow host dataset for IVF-PQ #1114

merged 4 commits into from
Jan 10, 2023

Conversation

tfeher
Copy link
Contributor

@tfeher tfeher commented Dec 21, 2022

This PR enables building (or extending) an IVF-PQ index using data in host memory.

@tfeher tfeher requested a review from a team as a code owner December 21, 2022 08:43
@tfeher tfeher added enhancement New feature or request non-breaking Non-breaking change improvement Improvement / enhancement to an existing function and removed python enhancement New feature or request labels Dec 21, 2022
@tfeher tfeher mentioned this pull request Dec 21, 2022
Open
@tfeher
Copy link
Contributor Author

tfeher commented Dec 21, 2022

Note for reviewer: ai_wrapper class is currently just a copy of cai_wrapper, and one shall reduce the redundant code in a follow up PR. I have created a separate issue for this: #1115.

cjnolet
cjnolet requested changes Jan 3, 2023
View reviewed changes
Copy link
Member

@cjnolet cjnolet left a comment

Choose a reason for hiding this comment

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

Looks great, just a couple small things.

python/pylibraft/pylibraft/common/ai_wrapper.py Show resolved Hide resolved
python/pylibraft/pylibraft/neighbors/ivf_pq/ivf_pq.pyx Outdated Show resolved Hide resolved
@tfeher tfeher force-pushed the enh_ivf_pq_host_dataset branch from 10be8a6 to 3d67acd Compare January 9, 2023 22:03
@github-actions github-actions bot added the python label Jan 9, 2023
tfeher
tfeher commented Jan 9, 2023
View reviewed changes
Copy link
Contributor Author

@tfeher tfeher left a comment

Choose a reason for hiding this comment

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

Thanks @cjnolet for the comments, I have updated the PR!

python/pylibraft/pylibraft/common/ai_wrapper.py Show resolved Hide resolved
python/pylibraft/pylibraft/neighbors/ivf_pq/ivf_pq.pyx Outdated Show resolved Hide resolved
cjnolet
cjnolet approved these changes Jan 9, 2023
View reviewed changes
Copy link
Member

@cjnolet cjnolet left a comment

Choose a reason for hiding this comment

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

Changes LGTM

@codecov-commenter
Copy link

codecov-commenter commented Jan 10, 2023 •
edited
Loading

Codecov Report

Base: 87.68% // Head: 87.99% // Increases project coverage by +0.30% 🎉

Coverage data is based on head (3408087) compared to base (9944b3a).
Patch coverage: 92.10% of modified lines in pull request are covered.

Additional details and impacted files 
@@               Coverage Diff                @@
##           branch-23.02    #1114      +/-   ##
================================================
+ Coverage         87.68%   87.99%   +0.30%     
================================================
  Files                20       21       +1     
  Lines               471      483      +12     
================================================
+ Hits                413      425      +12     
  Misses               58       58
Impacted Files Coverage Δ
python/pylibraft/pylibraft/common/ai_wrapper.py 88.88% <88.88%> (ø)
python/pylibraft/pylibraft/common/__init__.py 100.00% <100.00%> (ø)
python/pylibraft/pylibraft/common/cai_wrapper.py 100.00% <100.00%> (+11.11%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@cjnolet
Copy link
Member

cjnolet commented Jan 10, 2023

/merge

@rapids-bot rapids-bot bot merged commit 74ef826 into rapidsai:branch-23.02 Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjnolet cjnolet cjnolet approved these changes

Assignees

@tfeher tfeher

Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change python
Projects
VS/ML/DM Primitives Release Board
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tfeher @codecov-commenter @cjnolet