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

IVF-Flat index splitting #1271

Merged
merged 33 commits into from
Mar 15, 2023
Merged

Conversation

lowener
Copy link
Contributor

@lowener lowener commented Feb 10, 2023

Refactor of ivf_flat::index to split the cluster data in separate buffers

@github-actions github-actions bot added the cpp label Feb 10, 2023
@lowener lowener self-assigned this Feb 10, 2023
@cjnolet cjnolet added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Feb 10, 2023
@cjnolet cjnolet added the FAISS label Feb 13, 2023
@lowener lowener marked this pull request as ready for review February 22, 2023 07:13
@lowener lowener requested a review from a team as a code owner February 22, 2023 07:13
Copy link
Contributor

@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 @lowener for the PR, it looks great overall! I have just a few comments to add.

cpp/include/raft/neighbors/ivf_flat_types.hpp Show resolved Hide resolved
cpp/include/raft/neighbors/ivf_flat_types.hpp Outdated Show resolved Hide resolved
cpp/include/raft/neighbors/detail/refine.cuh Outdated Show resolved Hide resolved
@achirkin achirkin added breaking Breaking change and removed non-breaking Non-breaking change labels Feb 24, 2023
@lowener lowener requested a review from a team as a code owner February 28, 2023 01:17
@github-actions github-actions bot added the ci label Feb 28, 2023
Copy link
Contributor

@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 @lowener for addressing the issues! I have just one question left regarding list template params. I would actually ask @achirkin to have a final look at those changes. Apart from that the PR looks good to me, pre-approving.

cpp/include/raft/neighbors/ivf_pq_types.hpp Outdated Show resolved Hide resolved
Copy link
Member

@ajschmidt8 ajschmidt8 left a comment

Choose a reason for hiding this comment

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

Approving ops-codeowner file changes

@github-actions github-actions bot removed the ci label Mar 7, 2023
@cjnolet
Copy link
Member

cjnolet commented Mar 15, 2023

/merge

@rapids-bot rapids-bot bot merged commit 03e26b5 into rapidsai:branch-23.04 Mar 15, 2023
@lowener lowener deleted the 23.04-flat-split branch March 15, 2023 15:29
lowener added a commit to lowener/raft that referenced this pull request Mar 15, 2023
Refactor of `ivf_flat::index` to split the cluster data in separate buffers
- Addressing rapidsai#1170.
- Following rapidsai#1249 in the index structure implementation.
- Adding serialization API with stream and filename overloads
- Moving `raft::spatial::knn::ivf_flat` namespace to `raft::neighbors::ivf_flat`

Authors:
  - Micka (https://github.com/lowener)
  - Corey J. Nolet (https://github.com/cjnolet)
  - Artem M. Chirkin (https://github.com/achirkin)

Approvers:
  - Tamas Bela Feher (https://github.com/tfeher)
  - AJ Schmidt (https://github.com/ajschmidt8)
  - Artem M. Chirkin (https://github.com/achirkin)

URL: rapidsai#1271
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change cpp FAISS improvement Improvement / enhancement to an existing function
Projects
Development

Successfully merging this pull request may close these issues.

5 participants