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

Cleanup faiss includes #1098

Merged
merged 2 commits into from
Dec 14, 2022
Merged

Conversation

benfred
Copy link
Member

@benfred benfred commented Dec 14, 2022

This mainly removes un-needed faiss includes - but also removes the KeyValueWarpSelect function which isn't being used anywhere, and replaces faiss::gpu::KeyValuePair with cub::KeyValuePair in warp_select_faiss.cuh

This mainly removes un-needed faiss includes - but also removes
the `KeyValueWarpSelect` function which isn't being used anywhere,
and replaces faiss::gpu::KeyValuePair with cub::KeyValuePair
@benfred benfred requested a review from a team as a code owner December 14, 2022 00:26
@benfred benfred added the non-breaking Non-breaking change label Dec 14, 2022
@github-actions github-actions bot added the cpp label Dec 14, 2022
@benfred benfred added improvement Improvement / enhancement to an existing function and removed cpp labels Dec 14, 2022
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.

Just one little thing and we don't have to hold up the PR for it.

return (value != b.value) || (key != b.key);
}
};
using cub::KeyValuePair;
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't need to hold up this PR but it would be nice to use raft::KeyValuePair here for consistency. I've been converting the cub::keyvaluepair to the raft version everywhere for consistency except where the cub version is absolutely necessary.

@github-actions github-actions bot added the cpp label Dec 14, 2022
@cjnolet
Copy link
Member

cjnolet commented Dec 14, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit a9e060d into rapidsai:branch-23.02 Dec 14, 2022
@benfred benfred deleted the faiss_include_cleanup branch January 20, 2023 04:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cpp improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
Development

Successfully merging this pull request may close these issues.

2 participants